intellij-idealaw-of-demeter

IntelliJ Idea's Law of Demeter inspection. False positive or not?


Suppose the next class

interface Thing {
  void doSomething();
}

public class Test {
  public void doWork() {
    //Do smart things here
    ...
    doSomethingToThing(index);
    // calls to doSomethingToThing might happen in various places across the class.
  }

  private Thing getThing(int index) {
    //find the correct thing
    ...
    return new ThingImpl();
  }

  private void doSomethingToThing(int index) {
    getThing(index).doSomething();
  }      
}

Intelli-J is telling me that I'm breaking the law of demeter since DoSomethingToThing is using the result of a function and supposedly you can only invoke methods of fields, parameters or the object itself.

Do I really have to do something like this:

public class Test {
  //Previous methods
  ...

  private void doSomething(Thing thing) {
    thing.doSomething();
  }

  private void doSomethingToThing(int index) {
    doSomething(getThing(index));
  }
}

I find that cumbersome. I think the law of demeter is so that one class doesn't know the interior of ANOTHER class, but getThing() is of the same class!

Is this really breaking the law of demeter? is this really improving design?

Thank you.


Solution

  • Technically, this is breaking the Demeter's laws. Though I would contest that private functions should be considered for LoD-F, as supposedly they are not accessible from outside. At the same time, it's not really breaking Demeter's laws, if 'thing' is owned by Test. But in Java, the only way to get to thing may be through a getter, which takes this back to that technicality (no clear separation between getter and action methods).

    I would say, do this:

    public class Test {
      private Thing getThing(int index) {
        //find the thing
        return thing;
      }
    
      private void DoSomethingToThing(Thing thing) {
        thing.doSomething();
      }
    
      private void DoSomethingToThing(int index) {
        DoSomethingToThing(getThing(index));
      }
    }
    

    Or, probably better, have the caller use thing directly. Which is possible if Test's function is to produce or expose things, rather than being the intermediary to manipulate thing.