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

Question

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.

Pawel Veselov · Accepted Answer · 2013-10-23T19:46:08.330

0

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.

edited Oct 23 '13 at 19:46

answered Oct 23 '13 at 19:27

Pawel Veselov

3,996
7
44
62

What do you mean have Thing expose methods that do stuff to it? since Thing is exposing `doSomething()`. Other than that It seems you're suggesting that I'll have to add the extra method then. Right? – superjugy Oct 23 '13 at 19:42
Sorry, fixed. I'm suggesting direct manipulation, and/or overloading, so it just looks more neat. – Pawel Veselov Oct 23 '13 at 19:49
Well the thing here is that Thing is really an interface and getThing instantiates the real Class. but the Thing is only used inside Test privately. I'll update the question trying to reflect this. – superjugy Oct 23 '13 at 19:56
Then you can concede that the function of Test is to produce /store/provide instances of Thing, and nothing else. – Pawel Veselov Oct 23 '13 at 21:22
Well it has that function but it does has other functions. You might be getting at that this should probably be two classes instead of one. That sounds interesting. I'll look into that. I'll accept your answer now. Thank you very much. – superjugy Oct 23 '13 at 21:26

score 0 · Answer 2 · answered Aug 10 '16 at 10:41

IntelliJ is not detecting object instantiation correctly.

Wikipedia (what IDEA links to) describes that you can call objects created in the current context.

That's what I do, but still I get the warning on getMajor():

Version version = Loader.readVersion(inputStream); // Instantiates a new Version

if (version.getMajor() != 2)
    throw new IOException("Only major version 2 is supported");

IDEA's inspection offers an option to ignore calls to 'library' methods. In my case, Loader.readVersion() is a library method, however it's located inside the current project (the project must be self-supporting). IDEA thinks it's not a library method.

Since the mechanism of this inspection is inadequate/incomplete/naive (like with MANY of IDEA's inspections btw), the only solution is disabling it and attempt to avoid these situations manually.

Well, in my example the instance was being created and used in the same class. In your case you are creating the instance outside of the class. But I agree. Also if you want to use any dependency injector, this will always happen since the injector will give you the created instance of the object you need. I ended up disabling this check. — superjugy, Aug 12 '16 at 02:54

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

2 Answers2