Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#1062 VariableNamingConventions: False positive for final variables that are no constants

PMD-Backlog
open
None
PMD
1-Blocker
Bug
2014-02-09
2013-01-21
Ingo von Otte
No

Hi,

currently the VariableNamingConventions rule requires that all static final variables must be named in upper case (e.g. MY_CONSTANT). However, according to the most code conventions (Java, Eclipse) only constants shall be named in upper case. We sometimes declare variables final although they are no constants. For example:

private static final Logger logger;

private static final HashMap<String,String> textCache = new HashMap<String,String>();

I would assume that a real constant is a final variable where the instance is immutable. Thus, I would only issue this warning, if the type of the variable is a primitive type or a wrapper for a primitive type (which is immutable).

What do you think about this potential rule change? If there is a consensus about the rule definition, i am willing to supply a patch.

Best regards,

Ingo

Discussion

  • Romain PELISSE
    Romain PELISSE
    2013-01-26

    Hi Ingo,

    Interesting debate indeed :)

    It's true that a variable called COUNTRY_MAP, for instance, holding a Map of countries, which should be somewhat immutable, would be perceived as a constant while being mutable. However, what I like is this convention is that it says to the developer that this object SHOULD NOT be modified. For instance, I would never do :

    COUNTRY_MAP.put("Sildavy",sildavy);

    While, I would have no issue (at least visually) writing the following piece of code:

    countryMap.put("Sildavy", sildavy);

    From this point of view, I would say that PMD flag make sense. What do you think ? What other people think ? :)

     
    • Ingo von Otte
      Ingo von Otte
      2013-02-01

      Hi Romain,

      thanks for your reply. I aggree that variables that should not be modified should have uppercase names, such as your example with COUNTRY_MAP. At the same time I have encountered cases, where static final variables are not meant to be constants, e.g.

      private static final Logger logger;
      private static final HashMap textCache = new HashMap();

      Unfortunately, PMD can not distinguish these cases. However, for MOST CASES, it can, like:

      private static final int MY_CONSTANT = 1;
      private static final String STRING_CONSTANT = "abc";

      In these cases, we know that the variable is definetly a constant, either because it is a primitive type or an immutable object type.

      Thus, with my proposed change, PMD would find almost all wrongly named constants while allowing final static object variables to have a lower case name if they are no constants and are also not meant to be constants. Of course a PMD flag would be possible, but to be honest I think that the rule would still be strict enough to spare the addition of yet another switch.

      If you are interested, I have attached an updated rule to this post (look for "****" to find the important parts).

       
  • Romain PELISSE
    Romain PELISSE
    2013-01-26

    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,4 +1,3 @@
    -
     Hi,
    
     currently the VariableNamingConventions rule requires that all static final variables must be named in upper case (e.g. MY_CONSTANT). However, according to the most code conventions (Java, Eclipse) only constants shall be named in upper case. We sometimes declare variables final although they are no constants. For example:
    
    • status: open --> pending
    • assigned_to: Romain PELISSE
     
  • Ingo von Otte
    Ingo von Otte
    2013-02-01

    I can not see the attachement, so I'll try it a second time.

     
  • Andreas Dangel
    Andreas Dangel
    2014-02-09

    • status: pending --> open
    • Module: --> PMD
    • Milestone: PMD-5.0.x --> PMD-Backlog
    • Priority: 1 --> 1-Blocker
    • Type: --> Bug
    • Affects version: -->