Make sure to read this post before raising a thread here:
Then tell us:
What language is this for?
Java
Which rule?
S1214
Why do you believe it’s a false-positive/false-negative?
The constant interface pattern refers to “the use of an interface solely to define constants, and having classes implement that interface in order to achieve convenient syntactic access to those constants”. However, SonarQube appears to be reporting this Code Smell in all cases where an interface has been declared and has only constants. Take the following:
class Config {
interface Properties {
String APP_NAME = "appName";
String DISPLAY_NAME = "displayName";
}
void someMethod() {
final var appName = System.getProperty(Properties.APP_NAME);
}
In the above case, no class is implementing the Properties interface as described in the above reference and yet the Code Smell is being triggered. I believe the Code Smell should be triggered only in the case where a class implements an interface that has only static constants defined.
Are you using
SonarQube - Community Edition
How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
See example above
To provide further commentary on why the use of an interface is preferable place to define static properties that call out for a dedicated namespace, if you use a class, you have to define remember to declare the class as final and declare a private constructor. The problem with that is, if you care about code coverage, you will always show gaps in coverage, because the private constructor cannot be called by a test. If you cheat and make a package protected constructor, you have to write a meaningless test to cover the constructor. Using enum offers different tradeoffs. You don’t have to remember to declare the enum as final or implement a private constructor, but you still end up with coverage problems that cannot be corrected unless you make a bogus case as in:
Hello there, and welcome to our community!
I don’t think this should be a FP for us, as this could be considered somewhat of an anti-pattern in some cases.
I think if you want to use it, you could just mark the issue as FP on Sonarqube and get rid of it.
Otherwise I would suggest to switch to an enum and ignore the <100% coverage, as we all are force to do sometimes!
Leonardo, Thank you for the reply, but “… as this could be considered somewhat of an anti-pattern in some cases” is not a very convincing argument. Can you provide a specific example?
I think the rule as defined on Wikipedia is a valid one, and I would prefer not to disable it.
As it relates to putting these constants in an enum, I do not at all like the notion that “we’re forced to accept less than 100% code coverage” especially when there is a completely reasonable solution using interfaces. I think using a pattern that results in less than 100% code coverage should be considered an anti-pattern.
Regarding the rule application currently in Sonar, as soon as you add any kind of static method to the interface, the warning disappears. How does adding a static method to the interface take something that was an anti-pattern and make it not an anti-pattern? For example, take the Vert.x HttpHeaders interface. That is primarily a collection of static constants that happens to have four static methods. Why would removing those four static methods suddenly make the use of that interface an anti-pattern?
The wiki page you mentioned about the “constant interface pattern” reports some of the reasons that are also behind this rule.
This pattern in my opinion does not bring a strong enough case to remove this rule, nor to change it.
In your specific case, if this pattern was picked to achieve your objective in the code, I would simply mark the code smell as “won’t fix” or “false positive”.