Clean Code : The Road Less Travelled

A Do's and Dont's list to writing clean code

Chapter 1 : What this is about.

It is not enough for the code to work. - Robert C. Martin (Uncle Bob)

And indeed, it is not. The code needs to be readable, easily understandable without being misled, concise and simple so much that any newbie could tell what the code does. It's important that the code needs to work and it's equally important for the code to be "maintainable". There's also a corelation between a clean code and a bug-free performant application. More on that later!

After many years into software engineering, I've gathered certain nuggets of knowledge and wisdom around the same thought. Having learnt from co-workers, sessions, books, blogs and mostly from my own mistakes, this blog serves to be a very summarized documentary (artifact if you must) on my accumulated lessons on how to write and make code clean.

While there are already materials out there that talks more on the philosophy and the mindset you'd need to posses, this blog takes in a more practical real-life examples to a clean code.

Chapter 2 : Meaningful Names.

2.1. Use verbs for function names.

Tip: The name should ONLY describe what the function will do. And hence, ask yourself the question "What will this function do?" and that would be your name.

DOs:

public void validateUser(User user);
public int calculateSum(int a, int b);

DONTs:

public void userValidated(User user);
public int getSum(int a, int b); // boderline, debatable

2.2. Prefix with "is" when using boolean type variables.

Tip: Always choose names that enable setting “false” by default.

DOs:

boolean isUserValid;

DONTs:

boolean validUser;
boolean isUserNotInvalid; // Avoid negation/double-negation in names

2.3. Use plurals for list / arrays.

DOs:

List<Integer> numbers;

DONTs:

List<Integer> listOfNumbers;
List<Integer> numberList; // boderline, debatable

2.4. Use proper names in callback too. Don't let it be 'x'.

DOs:

numbers.stream()
      .filter( number -> number % 2 == 0 )
      .forEach( evenNumber -> doSomething( evenNumber ) );

DONTs:

numbers.stream()
      .filter( i -> i % 2 == 0 )
      .forEach( i -> doSomething( i ) );

2.5. Use specific names whenever wherever possible.

DOs:

User admin;
User betaUser;

DONTs:

User someUser;
User user; // can't help sometimes and that's understood

Chapter 3 : Functions - Simple and SOLID!

Coming to functions, the idea here is to keep things simple.

3.1. Have the function do just ONE thing.

An oversimplified example below I think, but puts across the point of having a single responsibility principle.

DO:

public void methodInServiceLayer(User user) {
    validateUser(user);
    sendUser(user);
}

DONT:

public void methodInServiceLayer(User user) {
    validateAndSendUser(user); // Break this into two
}

3.2. If there are a lot of arguments for the function, time to refactor!

Sometimes, the function keeps growing in terms of the number of arguments. Ideally keep it to maximum to 7 parameters. Beyond that, consider doing the following:

3.2.1. Either, group the arguments into a wrapper.

Let's take an example of the below method:

public User createUser(String firstName, String middleName, String lastName, int age, String  addressLine1, String addressLine2, String postcode, String city, String country);

This method is outrageously long! And this is just a simple example. Imagine if we were to add the dateOfBirth and more details! One approach would be to group the parameters into relevant wrappers. For example -

public class Name {
    String firstName;
    String middleName;
    String lastName;
}

public class Address {
    String addressLine1;
    String addressLine2;
    String postCode;
    String city;
    String country;
}

And now the function definition would look like below:

public User createUser(Name name, int age, Address address);

Beautiful, isn't it?

3.2.2. Or, if the function is about constructing an object, use Builder/Factory Pattern!

The other way around it is to make use of Builder or Factory Patterns. When the function is about constructing an object (like the above example), make use of the Builder pattern.

Chapter 4 : Return-Early Pattern for the Arrow-Antipattern.

One thing that really confused me when I was learning to program is the super nested if-else and loop blocks ! It still does! It got me spending a lot of time finding the close bracket of a particular if-clause or else-clause. A typical example of deeply nested conditions and loops would look like an 'arrow' (hence called the arrow anti-pattern!).

To solve this, enter 'Return Early Pattern'.

Return early is the way of writing functions or methods so that the expected positive result is returned at the end of the function and the rest of the code terminates the execution (by returning or throwing an exception) when conditions are not met.

The problem:

public String returnStuff(SomeObject argument1, SomeObject argument2) {
    if (argument1.isValid()) {
        if (argument2.isValid()) {
            SomeObject otherVal1 = doSomeStuff(argument1, argument2)

            if (otherVal1.isValid()) {
                SomeObject otherVal2 = doAnotherStuff(otherVal1)

                if (otherVal2.isValid()) {
                    return "Stuff";
                } else {
                    throw new Exception();
                }
            } else {
                throw new Exception();
            }
        } else {
            throw new Exception();
        }
    } else {
        throw new Exception();
    }
}

The solution:

public String returnStuff(SomeObject argument1, SomeObject argument2){
      if (!argument1.isValid()) {
            throw new Exception();
      }

      if (!argument2.isValid()) {
            throw new Exception();
      }

      SomeObject otherVal1 = doSomeStuff(argument1, argument2);

      if (!otherVal1.isValid()) {
            throw new Exception();
      }

      SomeObject otherVal2 = doAnotherStuff(otherVal1);

      if (!otherVal2.isValid()) {
            throw new Exception();
      }

      return "Stuff";
}

Notice how you could now read it clearly in a linear flow and isn't so confusing?

Chapter 5 : Say NO! to Zombie Code.

5.1. Never commit commented code.

More than often, I see code like below:

public boolean validateUser(User user) {
    ...some logic
    // We no longer do this because of some good reason
    // if( user != otherUser) {
    //     return false;
    // }
    return true;
}

The function should look like this:

public boolean validateUser(User user) {
    ...some logic
    return true;
}

But you might think, "What if we need that logic back?" Well, there is something called "git diff" for that and you would be able to get back the commented code.

5.2. Never commit unreachable code.

This is something intrinsic to your IDE these days. They usually alert you if there are any unreachable code that you might be trying to commit. But having said that, it is something to keep in mind.

Chapter 6. Comments - No comments.

Some love them, some not so much! There's only a thin line between the comments being helpful vs misleading. In my mind, good code doesn't need to be explained how it works. A general rule of thumb is to only have comments when something isn't so obvious.

Chapter 7. NPE - The Null Pointer Evil

It might just be the 'Resident Evil' since it's the result of your own unkempt code. Well, time to change that. Just follow the tips below and you'll be good!

7.1. Use String Constants as the first param of equals().

"Ronit".equals(user.getFirstName())
// or better, always have the string literals as a final static constant
USER_FIRST_NAME.equals(user.getFirstName())

7.2. Prefer the use of 'forEach' to 'for' loops.

The issue with the below

for(int i=0; i<users.size(); i++) {}

is that it might just happen that "users" is null. That would mean NPE! Also there runs a risk of accessing the wrong index which might fall outside the bounds of the list/array. A better approach would be to replace it with a forEach loop.

for(User user : users ) {}

7.3. Avoid null-checks via good practice.

Normally, a method that deals with non-primitive input needs to put validations that check for if the input is null. However, this creates a lot of unnecessary noise in the validation logic or the very method, making it harder to read. The philosophy here is to not have these null-checks at all by following good coding practice. A bit too ideal but something the project contributors should strive towards.

7.3.1. Explore and exploit 'Optional'.

Java 8 introduces Optional that provides a really good way to deal with the nulls. For example

public String pickFruitByFirstLetter(final List<String> fruits, final String firstLetter) {
  String result= null;
  for (String fruit: fruits) {
    if (fruit.startsWith(firstLetter)) {
      result= fruit;
      break;
    }
  }
  return result != null ? luckyName : "No Fruit found";
}

This null check can be replaced with the Optional class method isPresent() as shown below:

public String pickFruitByFirstLetter(final List<String> fruits, final String firstLetter) {
   final Optional<String> result = fruits.stream().filter(fruit-> fruit.startsWith(firstLetter)).findFirst();

   return result.isPresent() ? result.get() : "No Fruit found";
}

The above code might not look like it's solving much (it's still the same check), however, the Optional class supports other ways that are superior to check nulls. Like the return statement could be -

return result.orElse("No Fruit found");

There are more stuff there. Explore!

7.3.2. Either throw an Exception or return empty.

Ideally again, you should be throwing an exception if you encounter an anamoly while processing the input. Should not ever return null. If the anamoly is something that is expected, then you could return an empty instance (for example, Collections.emptyList()). However, consistency is important. This is something that should be enforced and abided by the project team.

7.3.3. Indicate in Javadoc if the method returns null.

Javadocs are not the source of truth, but if we were to be a good citizen, we should update the javadocs to state clearly what the method returns at what circumstance.

7.3.4. @Nullable, @NonNull.

Marking parameters as @Nullable and @NonNull gives a big advantage in dealing with NPE. Either the IDE could alert you on whether a method call has a null input or various static analysis tools could warn on the same. You could also use them on fields for DTOs/POJOs and enforce them on their builders/constructors to throw Exceptions if the input field doesn't hold up the agreement.

7.3.5. Use primitives over their wrappers.

Always prefer to use primitives over their wrapper whenever possible. Of course on account of wrappers being able to be null while primitives cannot be.

Did you find this article valuable?

Support Ronit's Blog by becoming a sponsor. Any amount is appreciated!