4

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?

John Smith
  • 6,129
  • 12
  • 68
  • 123
  • 1
    What if you changed Point to be a 3D point, you now have to change every class that uses a Point? This seems like a backwards move - IMHO. – Nigel Ren Oct 07 '17 at 16:34
  • 1
    @NigelRen changing a 2D point into a 3D point makes relay no sense IMHO. If he needs a 3D point he could inherit a 3D Point from the 2D Point by `class Point3D extends Point` – Webdesigner Oct 07 '17 at 16:40
  • 1
    Even if you do define a sub-class, how is this catered for in the second design? As for changing a 2D to 3D point - surely the whole point is you don't care what a Point is - I should be able to change it to a 15D point and the first design works, the second one fails! – Nigel Ren Oct 07 '17 at 16:53

3 Answers3

1

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();
Webdesigner
  • 1,954
  • 1
  • 9
  • 16
1

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.

Stefan
  • 1,325
  • 12
  • 25
  • I would argue that just because you assign the label "value object", that does not exempt the object from following good design practices. The Law of Demeter does apply to Value Objects too, as it applies to every object. – Robert Bräutigam Oct 10 '17 at 07:42
  • @RobertBräutigam: Considering the article you mentioned in your answer, the value object in itself is not the problem. It could have a method distanceToPoint(Point) that would "do something useful". Circle would need the same method to calculate the distance to a point or to another circle, instead of making the center point available. This is where I share the concerns in the discussion of your article. Being able to pass a Point, which provides usefull methods, rather than a container for X and Y, in the end seems useful to me. – Stefan Oct 10 '17 at 08:23
1

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".

Robert Bräutigam
  • 7,514
  • 1
  • 20
  • 38
  • Excellent article. Especially the first couple of posts in the discussion. Thanks for sharing. – Stefan Oct 10 '17 at 08:14