Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to spec a Class with Dynamic methods? #34

Closed
raul782 opened this issue Jul 7, 2013 · 3 comments
Closed

How to spec a Class with Dynamic methods? #34

raul782 opened this issue Jul 7, 2013 · 3 comments

Comments

@raul782
Copy link

raul782 commented Jul 7, 2013

Moved from phpspec issue #149
Say if I have an class that uses magic setter and getters, like in Doctrine with the class Persistent Object.

abstract class BaseEntity
{
    public function __call($method, $args)
    {
        $command = substr($method, 0, 3);
        $field = lcfirst(substr($method, 3));
        if ($command == "set") {
            $this->set($field, $args);
        } else if ($command == "get") {
            return $this->get($field);
        } else {
            throw new \BadMethodCallException("There is no method ".$method." on ".get_class($this));
        }
    }
} 


class EntityA extends BaseEntity
{
    protected $memberA;
}

class ServiceWithEntity
{

    protected $entity;

    public function setEntity($entity)
    {
        $this->entity = $entity;
    }

    public function execute()
    {
       return $this->entity->getMemberA();
    }
}

If I Spec to access this member

   /**
     * @param D4m\PhpSpecPlayGround\Entity\EntityA $entityA
     */
    function it_should_return_response_when_executing_service($entityA)
    {
        $entityA->getMemberA()->shouldBeCalled()->willReturn("memberA");
        $this->setEntity($entityA);
        $response = "memberA";

        $this->execute()->shouldReturn($response);
    }

I got this error , How could I make Prophect/PhpSpec aware of those dynamic methods?

D4m/PhpSpecPlayGround/Service/ServiceWithEntity     
  18  ! it should return response when executing service
      method `Double\D4m\PhpSpecPlayGround\Entity\EntityA\P51d882bcb03ab::getMemberA()` is not defined.

Repository testing these issues are here: https://github.com/raul782/phpspec-playground/

@everzet
Copy link
Member

everzet commented Jul 7, 2013

Prophecy tells you that getMemberA is not a part of your public API, which it is definitely not. In order to mock communication, you need to have a protocol (public API) for that communication in the first place & __call is not the way you define public APIs.

It's both technically hard and logically wrong for mocking tool AND for human beings to understand communication based on magic methods. That's the reason why this kind of object communications considered bad practice.

So in this particular case, mocking framework forces you to make implicit communication pattern explicit - through definition of explicit signatures. Is it bad or good - for you to decide, but in most cases magic methods cause more harm than good.

@everzet everzet closed this as completed Jul 7, 2013
@raul782
Copy link
Author

raul782 commented Jul 7, 2013

Thanks for your reply, yeah I think __call is not a way to define a public API, and I think I uncommented the wrong method. (Edited above) It should have been:

return $this->entity->getMemberA();

However, I understand internally, there is __call method working with it, so not good either.

The reason within in my context, is that I'm building a php component that works with a large Xml API, and I'm creating classes to match those Types exposed by the API and they have lots of properties, around 100 per type.

So explicitly writing getters and setters are a pain, right now, and __call is used to alleviate that load.
So, I will see if I can write a generator tool to write those methods, but didn't have the time for now.

@antonCPU
Copy link

antonCPU commented Aug 1, 2013

@everzet Could PhpDoc block be checked for magic methods declaration?
I've adjusted Prophecy\Doubler\Generator\ClassMirror to do this and seems everything worked well.
Not sure why limit Prophecy abilities if the case can be solved by several lines of code? Moreover it would enforce to have PhpDoc block for magic stuff, which I think is a good practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants