Law of Demeter confusion with the simple classes

Question

I'm working on computational geometry project. I have classes representing geometrical objects: Point, LineSegment and class, which performs calculations on those objects: Geometry. I'm confused with the Law of Demeter and "MethodWithPointAndLineSegment".

class Point
{
    public int X { get; set; }
    public int Y { get; set; }  
    ...
}

class LineSegment
{
    public Point InitialPoint { get; set; }
    public Point TerminalPoint { get; set; }
    ...
}

class Geometry
{
    private ... MethodWithThreePoints(Point p1, Point p2, Point p3)
    {
        // accessing X and Y properties of passed points
        ...
    }

    public ... MethodWithPointAndLineSegment(Point p1, LineSegment segment)
    {
        MethodWithThreePoints(p1, segment.InitialPoint, segment.TerminalPoint);
        ...
    }
}

My question is: does MethodWithPointAndLineSegment violates the Law od Demeter? I suppose yes, because it accesses InitialPoint and TerminalPoint properties and pass them as parameters to the MethodWithThreePoints, which accesses X and Y properties of those points. In the other words, MethodWithThreePoints uses properties of properties of objects passed to the class method.

If it violates the Law of Demeter, then I can't see best and reasonable solution for this problem. I know, I can add additional properties to the LineSegment class to satisfy LoD:

class LineSegment
{
    ...
    public int InitialPointX 
    { 
        get { return InitialPoint.X; }
        set { InitialPoint.X = value; }
    }
    //etc...
}

But when I want to invoke MethodWithThreePoints within MethodWithPointAndLineSegment it forces me to create new points: new Point(segment.InitialPointX, segment.InitialPointY)... and pass those new points to the MethodWithThreePoints. It introduces some additional and unwanted performance cost, because I have to create new objects and pass to them constructors values returned by the multi-level accessors.

I'm not sure, what would be the best solution for this problem and for many similar problems: satisfy LoD or convenience and in this case performance (those methods will be invoked many times in a short period for performing algorithms calculations).

Any suggestions and explanations are welcome.

send LoD to hell, of course you have to acess inner objects of objects, a segment is two points : a points is N coordinates (for N dimension), its easier to use MethodWithPointAndLineSegment(Point p1, LineSegment segment) than MethodWithThreePoints(Point p1, Point p2, Point p3) — Boo, Mar 21 '16 at 16:10
It's the "Law" part that is confusing. It's a guideline like all the others, and you seem to be proficient enough in OOD to understand in which cases it is more of a liability than a benefit. — Rotem, Mar 21 '16 at 16:12
The LoD is about hiding implementation details, not data structure. So you don't want thing.subServiceX.subServiceY.doSomething(). But user.contactInfo.address.street is not generally considered a violation; it would be silly to have to completely flatten all data structures. — Dax Fohl, Mar 21 '16 at 16:44

score 1 · Accepted Answer · answered Mar 21 '16 at 16:37

I would agree to say LoD is, as any good practices, a guideline you must adapt to your context.

I will certainly not tell you to send it to hell though. It reveals problems in your design.

If you are familiar with OOP, you know you should design highly coherent class which combine data and behavior, and tell them what to do (see Martin Fowler: http://martinfowler.com/bliki/TellDontAsk.html)

I have not enough information here to help you much, but I can tell you your Point and LineSegment class are simple POCO (no behaviour, only public get/set). It is only data without behavior, which is not really OOP friendly. It explains why you are tempted to manipulate this data in a service (even if you call your service "Geometry").

I Guess in your code Point and LineSegment represent the data, and Geometry represent the behavior you'd like to have.

A more OO design, let's say to add a translation behavior could be something like:

class Point : ITranslate
{
    public Point(int x, int y)
    {
        Y = y;
        X = x;
    }

    public int X { get; private set; }
    public int Y { get; private set; }

    public Point Translate(Translation translation)
    {
        //return a new translated point
    }
}

See how I keep my data for myself and just exposed the required behavior. Here I also design an immutable point, but we could also do something like this:

class Point : ITranslate
{
    public Point(int x, int y)
    {
        Y = y;
        X = x;
    }

    public int X { get; private set; }
    public int Y { get; private set; }

    public void Translate(Translation translation)
    {
        //apply the translation to myself internally on my X and Y
    }
}

Of course I would need more context to be more helpful, but I hope it already answers your question.

Just to be clear, I don't say you absolutely have to do that here, I just explain why you find it hard to respect Demeter in your case.

In my case Point and Segment are just data structures (representing pair of numbers and points) rather than objects with behaviour. I think, that performing calculations on the sets of Points and Segments using specific algorithms are not part of the single Point or Segment object behaviour. That's why I created separate class (Geometry) for performing calculations on geometrical objects, which need access to the inner objects of Points, Segments and others similar objects. — Dariusz, Mar 21 '16 at 16:52
Why do they need to access it? Cant' they ask this structure to do what they're suppose to do? — Ouarzy, Mar 21 '16 at 18:29
E.g. X and Y are necessary to calculate distance or vector product and vector is representing as LineSegment (with InitialPoint and TerminalPoint). Distance e.g. is necessary to find least distant pair of points from given array of Points. Now I think, You may have right, some calculations between single objects can be done by Point, LineSegment, etc. But calculations on the sets of Points, LineSegments, etc. can be done by external class like Geometry, which can invoke methods on Points, LineSegments, etc. I will consider it. — Dariusz, Mar 21 '16 at 18:57
That's the idea, but clearly I would need the full context to be more pertinent. If you have only one thing to keep it would be: you could not always follow good practices, but you have to be aware of why and what are the trade off. — Ouarzy, Mar 22 '16 at 10:00
This answer helped me realize, that I can design my classes better, by adding functionality to geometrical objects, instead of delegating everything to the `Geometry` class (so +1). However, there are still situations in which directly accessing X and Y of point, that belongs to some object is just simpler and smarter. Consider method within `LineSegment` class: `double VectorProduct(LineSegment other)`. Here I need X and Y of the endpoints of `other`. I think, it's bad idea to define additional properties and methods for each point, that belong to some object, so I will access them directly. — Dariusz, Mar 26 '16 at 10:07

Dax Fohl · Answer 2 · 2016-03-22T15:55:21.673

You could look at using F# for this. By using a separate Geometry class you're halfway there already.

F# is fundamentally built on the idea of immutable data. So instead of modifying your existing objects, any "modification" actually returns a new object. Yes there's a performance hit, but it's often not much; HFT algorithms need to be fast but frequently use F#/Haskell/OCaml because the slowdown is fairly negligible and the resulting code is much easier to reason about.

Your code in F# would look something like

type Point = {X: int; Y: int}
type Line = {Start: Point; End: Point}
module Geometry =
  // Some specific examples
  let MiddleOfThreePoints p1 p2 p3 = {X=(p1.X + p2.X + p3.X)/3; Y=(p1.Y + p2.Y + p3.Y)/3}
  let MiddleOfLineAndPoint l p = MiddleOfThreePoints l.Start l.End p
  // You can even use currying to shorten that:
  let MiddleOfLineAndPoint l = MiddleOfThreePoints l.Start l.End

  // Note this returns a *new* point, doesn't modify the existing
  let TranslatePoint x y p = {X = p.X + x; Y = p.Y + y}
  // This returns a *new* list of points; doesn't modify anything
  let TranslatePoints x y points = points |> Seq.map(fun p -> TranslatePoint x y p)
  // A shorter version by using partial application on TranslatePoint
  let TranslatePoints x y points = points |> Seq.map(TranslatePoint x y)
  // An even shorter version by using currying
  let TranslatePoints x y = Seq.map(TranslatePoint x y)

  // "Modify" your object using "with" syntax (creates new object, but convenient syntax)
  let SetY p y = { p with Y = y }

Immutability has some great advantages when talking about multithreaded access (no worry about concurrent mods because there aren't mods), being able to undo changes (just keep a list of previous versions), and understanding what's going on (nothing is going on). Plus the F# syntax is nice and concise.

F# allows you to cheat and set things as mutable when you need to, and to create regular classes/interfaces, but in general you shouldn't especially when doing algorithmic work.

Law of Demeter confusion with the simple classes

2 Answers2