3

I have a line of code looking like this:

String someString = "something"; if (Foo.SOME_CONSTANT_STRING.equals(someString))

which results in a violation: "Potential violation of Law of Demeter (static property access)"

What is the suggested approach here?

(Edit: I changed the code example)

Linus Fernandes
  • 498
  • 5
  • 30
Alix
  • 2,630
  • 30
  • 72

4 Answers4

2

The problem with accessing static variables is that you are enforcing an external state to the class that is hard to test. You should access it via a class variable such as:

private final Foo SOME_CONSTANT = Foo.SOME_CONSTANT_STRING;
public void doSomething(){
    String someString = "something";
    if (SOME_CONSTANT.equals(someString)){
        doTheWave();
    }
}

this, with a getter for SOME_CONSTANT, allows to test the "initial state" of the function more precisely.

bunyaCloven
  • 313
  • 1
  • 4
  • 14
  • It will be useful for testing purpose only or it has any other advantage as well. As we have to declare already declared constants again,writing extra code. – Gaurav Parek Feb 02 '18 at 13:49
1

You should write

if ("hello".equals(Foo.SOME_CONSTANT_STRING))

because as far as PMD can know, Hello is for sure non null when Foo.SOME_CONSTANT_STRING might not be.

Also, check out Wikipedia's page on Law of Demeter for a better understanding of it.

Seb - SonarSource Team
  • 942
  • 4
  • 9
  • My example was a bit simplified. "hello" was a string variable I think – Alix Oct 12 '15 at 13:08
  • How can PMD think that `Foo.SOME_CONSTANT_STRING` could be `null` when it's `final` and set to a value? I thank you for the reference but I'm not getting any wiser when it comes to the particular case of static property access. The only guess I have at this point is that if Foo was the calling class, I would not get this violation. – Alix Oct 12 '15 at 22:05
  • knowing that `Foo.SOME_CONSTANT_STRING` is a non `null` constant requires to resolve it. Which Findbugs might not do when its not in the current class. Looking at your question, I can not even tell it myself. I can assume it because its upper case and Foo seems to be a class, but I can't know it. – Seb - SonarSource Team Oct 13 '15 at 08:59
  • Let's say that you are right. How am I supposed to compare these two strings (one constant and one string variable) in a way that would not trigger violations? I don't want to move the constant to the invoking class , `Foo`, as it does not really belong there, but it seems to be the only way. – Alix Oct 18 '15 at 10:42
  • You mean to say that the variable should be first in the comparison? – Alix Oct 19 '15 at 08:22
  • Yes, but `"hello"` is really not a variable. It's a real constant from the language point of view. – Seb - SonarSource Team Oct 19 '15 at 08:24
  • Context and details is very important so I looked into my code to get the example as accurate as possible this time (without "simplification"): `private boolean isXCharacteristic(BluetoothGattCharacteristic characteristic) { return BleUuid.X_CHARACTERISTIC.equals(characteristic.getUuid()); }` If I switch the two compared objects I get the method chain calls LOD violation instead. – Alix Oct 19 '15 at 08:33
  • How about: `private boolean isXCharacteristic(BluetoothGattCharacteristic characteristic) { return BleUuid.isX_CHARACTERISTIC(characteristic.getUuid()); }`? This means, creating a static method, which calls equal behind the scenes and not accessing the property at all... – adangel Nov 19 '15 at 20:59
0

@Alix

This question was pretty old and there are many responses, but i think the correct answer was missing so am putting down my answer:

    SOME_CONSTANT_STRING should be declared final

Additionally as mentioned by other users earlier, a String.equals call on a null String object can result in an NPE (Null Pointer Exception) and such situations are best avoided in applications so that there is no unpredictable behavior. A null argument to String.equals is completely acceptable. Hence if we want to use String constants (objects) to perform such comparisons, we should make those constants final.

deepak
  • 76
  • 1
  • 5
0

Frankly, I think a static property in another class ought to be treated as a global constant which is accessible and allowed by Law of Demeter. I don't see why it has to be defined as a class variable always except to get around the rule implementation in PMD and , perhaps, to avoid duplication of the double or triple dot accessors everywhere in the class. A static import might mitigate it in some cases.

https://github.com/pmd/pmd/issues/2179

I have raised the above issue for the same.

Linus Fernandes
  • 498
  • 5
  • 30