Common code violations in Java

At work, recently I did a code cleanup of an existing Java project. After that exercise, I could see a common set of code violations that occur again and again in the code. So, I came up with a list of such common violations and shared it with my peers so that an awareness would help to improve the code quality and maintainability. I’m sharing the list here to a bigger audience.

The list is not in any particular order and all derived from the rules enforced by code quality tools such as CheckStyle, FindBugs and PMD.

Here we go!

Format source code and Organize imports in Eclipse:

Eclipse provides the option to auto-format the source code and organize the imports (thereby removing unused ones). You can use the following shortcut keys to invoke these functions.

  • Ctrl + Shift + F – Formats the source code.
  • Ctrl + Shift + O – Organizes the imports and removes the unused ones.

Instead of you manually invoking these two functions, you can tell Eclipse to auto-format and auto-organize whenever you save a file. To do this, in Eclipse, go to Window -> Preferences -> Java -> Editor -> Save Actions and then enable Perform the selected actions on save and check Format source code + Organize imports. Avoid multiple returns (exit points) in methods:

In your methods, make sure that you have only one exit point. Do not use returns in more than one places in a method body.

For example, the below code is NOT RECOMMENDED because it has more then one exit points (return statements).

private boolean isEligible(int age){
  if(age > 18){
    return true;
  }else{
    return false;
  }
}

The above code can be rewritten like this (of course, the below code can be still improved, but that’ll be later).

private boolean isEligible(int age){
  boolean result;
  if(age > 18){
    result = true;
  }else{
    result = false;
  }
  return result;
}


Simplify if-else methods:

We write several utility methods that takes a parameter, checks for some conditions and returns a value based on the condition. For example, consider the isEligible method that you just saw in the previous point.

private boolean isEligible(int age){
  boolean result;
  if(age > 18){
    result = true;
  }else{
    result = false;
  }
  return result;
}

The entire method can be re-written as a single return statement as below.

private boolean isEligible(int age){
  return age > 18;
}


Do not create new instances of Boolean, Integer or String:

Avoid creating new instances of Boolean, Integer, String etc. For example, instead of using new Boolean(true), use Boolean.valueOf(true). The later statement has the same effect of the former one but it has improved performance.

Use curly braces around block statements.

Never forget to use curly braces around block level statements such as if, for, while. This reduces the ambiguity of your code and avoids the chances of introducing a new bug when you modify the block level statement.

NOT RECOMMENDED

if(age > 18)
  result = true;
else
  result = false;

RECOMMENDED

if(age > 18){
  result = true;
}else{
  result = false;
}


Mark method parameters as final, wherever applicable:

Always mark the method parameters as final wherever applicable. If you do so, when you accidentally modify the value of the parameter, you’ll get a compiler warning. Also, it makes the compiler to optimize the byte code in a better way.

RECOMMENDED

private boolean isEligible(final int age){ ... }


Name public static final fields in UPPERCASE:

Always name the public static final fields (also known as Constants) in UPPERCASE. This lets you to easily differentiate constant fields from the local variables.

NOT RECOMMENDED
public static final String testAccountNo = '12345678';

RECOMMENDED
public static final String TEST_ACCOUNT_NO = '12345678';,

Combine multiple if statements into one:

Wherever possible, try to combine multiple if statements into single one.

For example, the below code;

if(age > 18){
  if( voted == false){
    // eligible to vote.
  }
}

can be combined into single if statements, as:

if(age > 18 && !voted){
  // eligible to vote
}


switch should have default:

Always add a default case for the switch statements.

Avoid duplicate string literals, instead create a constant:

If you have to use a string in several places, avoid using it as a literal. Instead create a String constant and use it.

For example, from the below code,

private void someMethod(){
  logger.log('My Application' + e);
  ....
  ....
  logger.log('My Application' + f);
}

The string literal “My Application” can be made as an Constant and used in the code.

public static final String MY_APP = 'My Application';

private void someMethod(){
  logger.log(MY_APP + e);
  ....
  ....
  logger.log(MY_APP + f);
}


Additional Resources:

Reference: Common code violations in Java from our JCG partner Veera Sundar at the Veera Sundar blog.

Do you want to know how to develop your skillset to become a Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you two of our best selling eBooks for FREE!

JPA Mini Book

Learn how to leverage the power of JPA in order to create robust and flexible Java applications. With this Mini Book, you will get introduced to JPA and smoothly transition to more advanced concepts.

JVM Troubleshooting Guide

The Java virtual machine is really the foundation of any Java EE platform. Learn how to master it with this advanced guide!

Given email address is already subscribed, thank you!
Oops. Something went wrong. Please try again later.
Please provide a valid email address.
Thank you, your sign-up request was successful! Please check your e-mail inbox.
Please complete the CAPTCHA.
Please fill in the required fields.

11 Responses to "Common code violations in Java"

  1. Javi says:

    I’m disagree with the last rule, If you use a duplicated String, JVM only create 1 object…. I think that the code isn’t better.

  2. Francois Montmasson says:

    NOT RECOMMENDED
    if(age > 18)
    result = true;
    else
    result = false;
    RECOMMENDED
    boolean result= age > 18;

  3. David Ha says:

    I don’t mind multiple returns since it means I have _less_ code to read. E.g.

    if (condition 1) {
    throw …;
    }

    // No else block required.

    if (condition 2) {

    // Stop reading here. No need to check whether a “result”
    // variable is subsequently modified.

    return …;
    }

    // No else block required.

    return …;

  4. I disagree with multiple return statements, they’re very useful, I use them often when I got what I wanted and the next code is not needed anymore

  5. Valentin Baca says:

    While it mark too may lines in a legacy codebase, I find it incredibly helpful to turn on more warnings under the “Unnecessary Code” section of Eclipse’s Preferences > Java > Compiler > Errors/Warnings.

    Also, word of warning to adjust the settings for formatting comments. Comments should almost always be left exactly as-is, but if Eclipse is allowed to format comments it can mangle them.
    In other words, spot test before just applying to your entire workspace.

  6. mike says:

    If you dont return early your methods have the worst case running time in every case.

  7. Edinei says:

    I disagree that there shoud be one exit point. It seems so procedural style. Martin Fowler says to remove control flag and use return instead in his refactoring book. http://sourcemaking.com/refactoring/remove-control-flag

  8. Guest says:

    If a method is long, multiple return statements will make it hard to understand what the method does. Using only one exit point will make the method easier to understand. I think this the reason for the “don’t use multiple return statements” rule.
    But the method is still long and therefore probably mixes different levels of abstraction, and today a good method should be short and only handle one level of abstraction. Because of this, you should refactor such a method to make it shorter (e. g. extract other methods). Then the method is so easy a single return statement just introduces more code you have to read. I would then use multiple return statements again.
    I think because of this the rule should be changed to: If a method has multiple return statements, make sure it is so short, that it is still easy to understand (If it is not, do not change it to use only a single return statement, but refactor it, so that it is shorter (and therefore easier to understand)).

  9. If a method is long, multiple return statements will make it hard to understand what the method does. Using only one exit point will make the method easier to understand. I think this the reason for the “don’t use multiple return statements” rule.
    But the method is still long and therefore probably mixes different levels of abstraction, and today a good method should be short and only handle one level of abstraction. Because of this, you should refactor such a method to make it shorter (e. g. extract other methods). Then the method is so easy a single return statement just introduces more code you have to read. I would then use multiple return statements again.
    I think because of this the rule should be changed to: If a method has multiple return statements, make sure it is so short, that it is still easy to understand (If it is not, do not change it to use only a single return statement, but refactor it, so that it is shorter (and therefore easier to understand)).

  10. Instead of using

    new Boolean(true);

    or using

    Boolean.valueOf(true);

    you could also use

    Boolean.TRUE;

Leave a Reply


× 3 = twenty one



Java Code Geeks and all content copyright © 2010-2014, Exelixis Media Ltd | Terms of Use | Privacy Policy | Contact
All trademarks and registered trademarks appearing on Java Code Geeks are the property of their respective owners.
Java is a trademark or registered trademark of Oracle Corporation in the United States and other countries.
Java Code Geeks is not connected to Oracle Corporation and is not sponsored by Oracle Corporation.
Do you want to know how to develop your skillset and become a ...
Java Rockstar?

Subscribe to our newsletter to start Rocking right now!

To get you started we give you two of our best selling eBooks for FREE!

Get ready to Rock!
You can download the complementary eBooks using the links below:
Close