Let’s Clean that Code

Reading Time: 8 minutes

Imagine that you are starting with a new project with your team. You have the client’s legacy code and now your task is to go through it. Glancing over the code, you realise your worst nightmare as a Developer. The code has no comments, no explanation of what a method is achieving, methods are hundreds of lines long, there is a vague concept of encapsulating related items in a space. Every thought going through your head is sure to be a cuss word. If that’s what is happening in your head, then the code that you are reading is not a Clean Code.

As eloquently noted by Robert Martin in his book “Clean Code”, the only valid measurement of code quality is the number of WTFs per minute. If that count has reached a good number by just going through one of the modules, then that code is not an example of Clean Code.

Code quality measurement

Why do we need Clean Code?

On more formal terms, the quality of a code directly correlates to the maintainability of the product. Under the pressure of a deadline, you might rush to go faster, leaving a messy code. You get your feature to work, but now a bug pops up. You go back to your messy code, and even you are not able to make any sense out of it. This add-on of debugging the messy code will generally take much more time than the amount of time you spent on writing the code. On the other hand, if you would have written a “cleaner” code, with a second glance you would have been able to tell what every line of your code is doing, and debugging would have been that much easier.

Clean Code v/s unclean code

Just keep in mind, Programmers are really authors, and their target audience should not be the computer, but other developers. If another developer can read your code without mocking you, then well you have written a clean code.

How to Write Clean Code ?

Meaningful Names

Every variable, method, class, package in our code has a name. So, naming each of them appropriately should be the first concern of a Good Programmer.

Use Intention Revealing Names

The name of a variable, function, or class, should answer all the big questions a developer might have. It should tell him why it exists, what it does, and how it is used. If the name requires a comment along with it, then the name does not reveal it’s intent.

As an example, if you want a variable for the elapsed time in days, then

int d; // elapsed time in days

is a bad piece of code. You might want to replace it with

int elapsedTimeInDays;
int daysSinceCreation;
Use Pronounceable Names
Proper Naming and Pronunciation is the key

As humans, a major portion of our brain is dedicated to the concept of words, and words are pronounceable. So, it’s better to take advantage of that huge portions of our brain and make names pronounceable.

For instance,

private Date genymdhms //generation year, month, date, hour, // min, sec

can easily be replaced by something as simple as

private Date generationTimestamp
Noun for Classes

Classes and Objects are entities or things. So, naming them using Nouns or Noun Phrase is appropriate. For a class that Parses Addresses and provides methods to parse an address, AddressParser works.

Verbs for Methods

Methods are doing something for you, and in English, verbs represent “Doing” something. So, if want to name a method that deletes a customer record, then naming it deleteCustomer() is a good choice.

Pick One Word for Concept

All over your code, you are bound to create a lot of “managers”, like a DeviceManager, ProtocolManager. So, in your code, if you have an abstract concept, choose one word for it. Use Manager, and not Controller.

Switch between Solution Domain Names and Problem Domain Names

The people who will read your code are programmers. So, if you want to use Computer Science terms, like algorithm names, pattern names, math terms, you can use them. But if there is no “programmer-ease” name for what you are implementing, you switch to a name from the problem name. In such a case, another programmer can ask a domain expert for its meaning.

Better Functions

The Smaller, the Better

Multiple functions of 3-4 lines are sometimes far better than a single 100 lines function, even though both of them are doing the same thing. The condition being that those multiple functions should be transparently obvious, with each of them having a story. Also, each function should lead you to the next in a compelling order.

One Level Indentation

To make the functions easier to read and understand, we can try to keep its indentation level up to two. This means, that any if-else block or a pattern match in a function should be a single line, preferably calling another function.

Do One Thing

You want your function to do only one thing, but how do you define what that one thing is? To start with, try describing your function by describing it as a brief TO paragraph. So, if you have a function called fetchCustomerDetails, it’s TO brief might look something like
TO fetchCustomerDetails, we test to see if the given customer ID exists in the database, and if so, we provide the customer details.

Now, if your function is doing only those steps that are one-level below the stated name of the function, then it is doing only one thing.

def fetchCustomerDetails(id: Long): Option[Customer] = {
if(doesCustomerExists(id))
    readCustomerDetails(id)
else None
}
Error Handling is One Thing

Functions should do one thing, and error handling is one thing. So, a function that handles errors should do nothing else. The very first word in such a function should be try and there should be nothing after the catch / finally blocks.

Separate out Commands and Queries

Functions should either do something or answer something. Either it should change the state of an object, or it should return some information about that object. It’s confusing when a function does both.

Consider a function that changes the state of a named attribute if it exists and returns True. If the attribute does not exist, it returns false.
def set(attribute:String, value:String):Boolean

This might seem fine, but when you use this method with if expression, then it might become ambiguous. if(set("name", "bob")) might mean to check if the existing value of the named attribute is “bob” or not, which is not what we want the method to achieve. To overcome this, it’s better to restructure the code as:

if(attributeExists("name")) setAttribute("name","bob")
More Arguments are Evil

When a function seems to need more than two arguments, some of those arguments ought to be wrapped into a class of their own. So, a method like

def makeCircle(double x, double y, double radius): Circle

can be written down as

def makeCircle(Point center, double radius): Circle
Don’t Repeat Yourself (DRY)

DRY is a basic principle of software development aimed at reducing repetition of information. In simple words, the aim is to divide our code into smaller reusable piece of codes, instead of writing the same lines again and again. This ensures that in case a change is required in the implementation of the repeated code, then it should be done only in a single place.

Error Handling

Get Rid of Checked Exceptions

A Checked Exception is when the signature of every method would list all of the exceptions that it could pass to its caller. Initially, it might look like a good idea, but beware of the fact that your code literally wouldn’t compile if the signature does match what your code could do.

The price of checked exceptions is an Open/Close principle violation. If you throw a checked exception from a method in your code and the catch is three levels above, you must declare that exception in the signature of each method between you and the catch. This means that a change at a low level of the software can force signature changes on many higher levels. So, it’s better to avoid them and go for try/catch enclosed blocks.

Define Exceptions in terms of the Caller’s needs

Consider a call to a third-party library, and in your code, try to cover all the exceptions that the call could throw. The code will look something like this:

try {
new ACMEPort(portNum).open()
} catch {
case e: Exception1 => reportPortError(e)
case e: Exception2 => reportPortError(e)
case e: Exception3 => reportPortError(e)
}

Now, if you look closely you will see that there is a lot of code duplication. Instead of making a bloated code, it is better to wrap the API call in your custom class and make sure that the method returns a common exception type:

case class LocalPort(portNum: Int) {   
private val innerPort: ACMEPort = new ACMEPort(portNum)   def open() = {
 try {
       innerPort.open()
 } catch {
   case e: Exception1 => throw CommonException(e)
                 case e: Exception2 => throw CommonException(e)
                 case e: Exception3 => throw CommonException(e)
         }
 } 

So, now whenever you’ll call the Third Party API in your code, you’ll only have to take care of only one CommonException.

Neither Return Null, nor Pass Null

No matter how much you are tempted to return Null from a method, don’t do it. As soon as you return Null, you increase your work to handle that Null. Also, testing that Null value thoroughly so that none of the operations spin out of control. Instead of returning Null, consider throwing an Exception, or better yet a Special Case Object. So, for a method that fetches Customer Details, if the details are not present you can try to return an empty CustomerDetails object to handle the scenario. This would clean up a lot of your checks for null. In Scala, this is handled in a much better way by returning an Option[CustomerDetails]

Test It

Clean Tests should follow the F.I.R.S.T principles:

Fast

Tests should be fast. When tests run slow, you will not want to run them frequently. And if you don’t run them frequently, you will not be able to find problems early enough to fix them. You won’t feel as free to clean up the code and eventually the code will begin to rot.

Independent

Tests should not depend on each other. One test should not set up the conditions for the next test. You should be able to run each test independently and run the tests in any order you like. When tests depend on each other, then the first one to fail causes a cascade of downstream failures, making diagnosis difficult and hiding downstream defects.

Repeatable

Tests should be repeatable in any environment. You should be able to run the tests in the production environment, in the QA environment, and on your laptop while riding home on the train without a network.

Self-Validating

The tests should have a boolean output. Either they pass or fail. You should not have to read through a log file to tell whether the tests pass

Timely

The tests need to be written in a timely fashion. Unit tests should be written just before the production code that makes them pass. If you write tests after the production code, then you may find the production code to be hard to test.

Conclusion

It takes an effort to write code, and a lot of more effort to write clean code. Writing clean code is hard work. It needs a lot of practice and focus during execution.

The hardest part to write a clean code is simply making a start, but as time goes by, it becomes easier. It is all about readability, so even the smallest things like changing your habits for naming variables makes the biggest difference.

We should be consciously trying to improve code quality and decrease code entropy. The boy scouts of America have a simple rule: “Leave the campground cleaner than you found it.” The same rule applies to programmers. Always leave the checked-in code cleaner than the code that is checked-out.

Knoldus-blog-footer-image

Leave a Reply