Through the eyes of sonar : Naming & Comments.

Fourth “Through the eyes of sonar” article ( 1, 2, 3 ), let’s review various naming & comments rules.

Guidelines

English: Illustration of all caps .vs. mixed case.

English: Illustration of all caps .vs. mixed case. (Photo credit: Wikipedia)

The excellent work of Cagdas Basaraner provide us these guidelines :

  • Use short enough and long enough variable names in each scope of code. Generally length may be
    • 1 char for loop counters,
    • 1 word for condition/loop variables,
    • 1-2 words for methods,
    • 2-3 words for classes,
    • 3-4 words for globals.
  • Use specific names for variables, for example "value", "equals", "data", … are not valid names for any case.
  • Use meaningful names for variables. Variable name must define the exact explanation of its content.
  • Don’t start variables with o_, obj_, m_, etc. A variable does not need tags which states it is a variable.
  • Obey company naming standards and write variable names consistently in application: e.g. txtUserName, lblUserName, cmbSchoolType, … Otherwise readability will reduce and find/replace tools will be unusable.
  • Obey programming language standards and don’t use lowercase/uppercase characters inconsistently: e.g. userName, UserName, USER_NAME, m_userName, username,
    • use Camel Case (aka Upper Camel Case) for classes: VelocityResponseWriter
    • use Lower Case for packages: com.company.project.ui
    • use Mixed Case (aka Lower Camel Case) for variables: studentName
    • use Upper Case for constants : MAX_PARAMETER_COUNT = 100
    • use Camel Case for enum class names and Upper Case for enum values.
    • don’t use ‘_’ anywhere except constants and enum values (which are constants).
  • Don’t reuse same variable name in the same class in different contexts: e.g. in method, constructor, class. So you can provide more simplicity for understandability and maintainability.
  • Don’t use same variable for different purposes in a method, conditional etc. Create a new and different named variable instead. This is also important for maintainability and readability.
  • Don’t use non-ASCII chars in variable names. Those may run on your platform but may not on others.
  • Don’t use too long variable names (e.g. 50 chars). Long names will bring ugly and hard-to-read code, also may not run on some compilers because of character limit.
  • Decide and use one natural language for naming, e.g. using mixed English and German names will be inconsistent and unreadable.
  • Use meaningful names for methods. The name must specify the exact action (ideally behavior) of the method and for most cases must start with a verb. (e.g. createPasswordHash)
  • Obey company naming standards and write method names consistently in application: e.g. getTxtUserName(), getLblUserName(), isStudentApproved(), … Otherwise readability will reduce and find/replace tools will be unusable.
  • Obey programming language standards and don’t use lowercase/uppercase characters inconsistently: e.g. getUserName, GetUserName, getusername, …
    • use Mixed Case for method names: getStudentSchoolType
    • use Mixed Case for method parameters: setSchoolName(String schoolName)
  • Use meaningful names for method parameters, so it can document itself in case of no documentation.

Sonar Naming Ruleset

Package Name

A package is lowercase and avoid strange character

Name ‘com.mestach.myBeans‘ must match pattern ‘^[a-z]+(\.[a-zA-Z_][a-zA-Z0-9_]*)*$‘.

 
package com.mestach.myBeans; //bad
package com.mestach.model;   //good

Either you repackage to com.mestach.mybeans or com.mestach.model
Note that windows isn’t case sensitive but linux is. So if you are using an old versioning system like CVS,
Renaming once committed to a linux from windows may be harder (packages end up as folder : com/mestach/mybeans vs com/mestach/myBeans)

Member Name

Member Name : Name ‘FIELD_FIRSTNAME‘ must match pattern ‘^[a-z][a-zA-Z0-9]*$‘.

 protected String FIELD_FIRSTNAME = "fieldfirstname";

Either
— it’s a constant not well defined → add static final
— it’s an attribute of the class and so it’s preferable to use the lowercase version : fieldFirstName

Local Variable Name and Parameter Name

Local Variable Name : Name ‘fromDate_db2TimeStamp‘ must match pattern ‘^[a-z][a-zA-Z0-9]*$

String fromDate_db2TimeStamp = DATEFORMAT_db2TimeStamp.format(fromDate);

Avoid underscore in variable name, java naming convention is more camelCase.

String fromDateDb2TimeStamp = DATEFORMAT_db2TimeStamp.format(fromDate);

Parameter and variable should starts by a lowercase :

public void setMember(Member Member) // BAD
 public void setMember(Member member) //GOOD

A more complex sample involving spring injection :

        //BAD
       @Autowired
       private MyService module_MyService;
      public void setModule_MyService(MyService module_MyService){
               this.module_MyService = module_MyService;
      }

Prefer

        //GOOD
        @Autowired
        @Qualifier("module_MyService")
        private MyService myService;
        public void setMyService(MyService myService){
            this.myService=myService;
       }

Static Variable Name

Sometime… it’s just a final which is missing :

private static Logger LOG = Logger.getLogger(BatchLogger.class);   // BAD
private static final Logger LOG = Logger.getLogger(BatchLogger.class); //GOOD

Often in the code base you have (open) enum like this classes

public class ObjectType {
   public static String SRVTYPE = "SERVER";
   public static String PRINTERTYPE = "PRINTER"
   public static ObjectType SERVER = new ObjectType(SRVTYPE );
   public static ObjectType PRINTER= new ObjectType(PRINTERTYPE );
   private String code;
   
   private ObjectType(String c) {
	code = c;
   }
   public String getCode() {
	return code;
    }
  }

Can be fixed by :

 public class ObjectType {
   private static final String SRVTYPE = "SERVER";
   private static final String PRINTERTYPE = "PRINTER"
   public static final ObjectType SERVER = new ObjectType(SRVTYPE );
   public static final ObjectType PRINTER= new ObjectType(PRINTERTYPE );


    ...

– Don’t publish you SERVER string “constant”, make them private
— The api using this ObjectType.SRVTYPE can be fixed by ObjectType.SERVER.getCode()

Malicious code vulnerability – Field isn’t final but should be

– Make your constant final ! It will prevent modifying the constant inadvertently

 
ObjectType.SERVER =ObjectType.PRINTER

Method Name

Starts with lower case and use camelCase

 //BAD
  public String ProductReasonForm(Model model) {

Prefer

 
//GOOD
  public String productReasonForm(Model model) {

“Name methods after what they are intended to do.” (rather than how they do it) Kent Beck

Bad practice – Class names shouldn’t shadow simple name of superclass

This would help other developper to import the “correct” implementation.

  public class DwrController extends org.directwebremoting.spring.DwrController

Just rename the class to avoid ambiguity.

  public class CustomDwrController extends org.directwebremoting.spring.DwrController

Bad practice – Confusing method names

Bad practice – Confusing method names : Confusing to have methods com.mestach.Person.getDeclarationWithincome() and com.mestach.Person.getDeclarationWithIncome()

Did you notice the “case changes in method name “: Withincome vs WithIncome

– either you wanted to override the method… and you mispelled it.
— either you should perhaps rename it with a less confusing name.

Use positive conditionals

Use positive conditionals. Readability of positive conditionals are better than negative ones.

Negated boolean variable names must be avoided.

 boolean isNotFound; // BAD
 boolean isNoError // BAD

Positive !

 boolean isError; // GOOD
 boolean isFound; // GOOD

The problem arise when the logical not operator is used and double negative arises.
It is not immediately apparent what !isNotError means.

Avoid boolean flags in method signature

Don’t use method alternatives with boolean flag variables (public void someMethod(bool flag)). Write more than one method for each flag condition.

Eg :

 Person loadPerson(Long id, boolean withContactInfos)

Prefer

 /** don't the load the ContactInfos by default */
 Person loadPerson(Long id )
 Person loadPersonWithContactInfos(Long id )

Or introduce an initializationMode

 enum PersonInitializationMode {
     WITHCONTACTINFOS, WITHACCOUNTNUMBERS
  }

    Person loadPerson(Long id ,PersonInitializationMode...  initMode)

More samples from Alex Ruiz’s boolean arguments can be evil

In summary, let’s not use boolean parameters. They don’t communicate their purpose. Instead, we can:
— in the case of methods, split them in two
— in the case of constructors, use static factory methods
— in the case of “three-state” booleans, use Java enums

Use understandable and long names for variables.

– Loop variable names can be i, j, k, index etc.,
— local variable names must be longer than loop variables, parameter names must be longer than local variables and static variable names must be longer than parameters; proportional with scope size

Prefer

 int currentLineNumber;
double amount;

over

 int cln;
 double a;

non-Static nested inner classes

If possible make them static :
— They are more loosely coupled to the parent class. In general, the weaker the coupling, the better.
— They do not hold a pointer to the parent instance. This means:
— They can exist without a parent instance.
— They have a slightly smaller memory footprint.
— They don’t prevent the parent from being GCed.
And ideally move them outside, this will make the code more readable.

A typical example where it causes issues : inner class that extends TimerTask gets registered with a Timer, and then the timer’s thread keeps both the TimerTask and the parent instance alive.

Avoid static methods

Think twice before defining a method as static and be sure if you really need to. Static methods are harder to manage :
— test (state, singleton) and
— evolve (methods overriding, use of beans, alternate implementation).

Comments

Prefer a good naming over comments

For example avoid abbreviation

  /** compute the average */
 double compAvg(double a ,double b)

The name of the method explain it’s purpose !

double computeAverage(double a ,double b)

A comment is often never maintained as the code is refactored.
Leaving the maintainer with comments that don’t match the reality anymore.

Bad comment is worse than no comment.

Don’t comment to delete code

Don’t use comment lines to delete code, just delete. Version controlling system will help you if deleted code is required.

Sonar can help you identifying this. In the comments widget you have the commented LOCs.

 Comments
 28.6%
 70,143 lines 
 74.6% docu. API
 4,604 undocu. API
1,770 commented LOCs
Enhanced by Zemanta
About these ads
  1. Through the eyes of sonar : Naming & Comments. | Software Quality - Sonar by SonarSource | Scoop.it
  2. Through the eyes of sonar : Collections. | Don't Make the Same Mistake Twice
  3. Through the eyes of sonar: recap. | Don't Make the Same Mistake Twice

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Follow

Get every new post delivered to your Inbox.

%d bloggers like this: