This also violates Demeter's law? Or it would be an overkill to warp it?

Viewed 96

a very simple point:

class Point
{
    private $x, $y;

    public function __constructor($x, $y)
    {
        $this->x = $x;
        $this->y = $y;
    }

    public function getX()
    {
        return $this->x;
    }

    public function getY()
    {
        return $this->y;
    }
}

and a circle

class Circle
{
    private $r;
    private $point;

    public function __constructor (Point $point, $r)
    {
        $this->point = $point;
        $this->r = $r;
    }

    public function getPoint() // here is the law breaking
    {
        return $this->point;
    }
}

$circle = new Circle(new Point(1,2), 10);
$circle->getPoint()->getX(); // here is the law breaking
$circle->getPoint()->getY(); // here is the law breaking

of course it breaks the Demeter's law. So let me refractor it:

class Circle
{
    private $r;
    private $point;

    public function __constructor (Point $point, $r)
    {
        $this->point = $point;
        $this->r = $r;
    }

    public function getPointX()
    {
        return $this->point->getX();
    }

    public function getPointY()
    {
        return $this->point->getY();
    }
}

$circle = new Circle(new Point(1,2), 10);
$circle->getPointX();
$circle->getPointY();

apart from it looks better, I dont see any advantage - just two extra wrapping function. Technically I again have full access to Point and there is no way that more methods would be added to Point. Is it still worth to use the 2nd refractored method?

3 Answers

Beside the fact that this is more a opinion based question, I would do it like this:

class Circle extends Point
{
    private $r;

    public function __constructor (Point $point, $r)
    {
        $this->x = $point->getPointX();
        $this->y = $point->getPointY();
        $this->r = $r;
    }
}

$circle = new Circle(new Point(1,2), 10);
$circle->getPointX();
$circle->getPointY();

I agree with @Webdesigner that this is rather an opinion based question.

However, in my opinion, I don't think breaking the law of demeter is really an issue here when you consider Point to be a value object.

The getter could be

public function center() : Point { ... };

to make it even more clear.

While the second version does not violate the Law of Demeter technically, I would argue it still violates it "in spirit". The reason you can not tell which one is better, is because the second one is only marginally better than the first one in terms of information hiding and encapsulation (the things the Law of Demeter tries to codify).

The Law is about hiding the internal structure of the object, thereby enabling encapsulation, information hiding and preventing coupling. Getters (accessors) by their very nature are the opposite of information hiding, they enable access to internal structures instead of preventing it.

Summary: the getters are your problem. As long as you have those, you will always have to fight the Law of Demeter, and you will probably lose :)

Ok, so how do you code without getters you ask? Well, you add methods that do something for you with the coordinates. Things that you need. Distance between points, transforming points, etc. Whatever you need in your current application. Think about what a Point means in your current context.

Check out the "The Genius of the Law of Demeter".

Related