Clean Code Function: Writing Efficient & Maintainable Code

Cost of Owning a Mess

If you have been a programmer for more than two or three years, you have probably been significantly slowed down by someone else’s messy code. If you have been a programmer for longer than two or three years, you have probably been slowed down by messy code. The degree of the slowdown can be significant. Over the span of a year or two, teams that were moving very fast at the beginning of a project can find themselves moving at a snail’s pace. Every change they make to the code breaks two or three other parts of the code. No change is trivial. Every addition or modification to the system requires that the tangles, twists, and knots be “understood” so that more tangles, twists, and knots can be added. Over time the mess becomes so big and so deep and so tall, they can not clean it up. There is no way at all.

What is Clean Code?

Bjarne Stroustrup, inventor of C++
I like my code to be elegant and efficient. The logic should be straightforward to make it hard for bugs to hide, the dependencies minimal to ease maintenance, error handling complete according to an articulated strategy, and performance close to optimal so as not to tempt people to make the code messy with unprincipled optimizations. Clean code does one thing well.

Dave Thomas,Founder of OTI
Clean code can be read, and enhanced by a developer other than its original author. It has unit and acceptance tests. It has meaningful names. It provides one way rather than many ways for doing one thing. It has minimal dependencies, which are explicitly defined, and provides a clear and minimal API. Code should be literate since depending on the language, not all necessary information can be expressed clearly in code alone.

Functions

Sample Function

Read through this function and let me know what you understand.

Do you understand the function after five minutes of study? Probably not. There’s too much going on in there at too many different levels of abstraction.

However, with just a few simple method extractions, some renaming, and a little
restructuring the above will look like this.

SMALL!

The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that.

BLOCKS AND INDENTING

This implies that the blocks within if statements, else statements, while statements, and so on should be one line long. Probably that line should be a function call. Not only does this keep the enclosing function small, but it also adds documentary value because the function called within the block can have a nicely descriptive name. This also implies that functions should not be large enough to hold nested structures. Therefore, the indent level of a function should not be greater than one or two. This, of course, makes the functions easier to read and understand.

DO ONE THING

Functions should do one thing. They should do it well. They should do it only.

SWITCH STATEMENTS

It’s hard to make a small switch statement. Even a switch statement with only two cases is larger than I’d like a single block or function to be. It’s also hard to make a switch statement that does one thing. By their nature, switch statements always do N things. Unfortunately, we can’t always avoid switch statements, but we can make sure that each switch statement is buried in a low-level class and is never repeated. We do this, of course, with polymorphism.

There are several problems with this function. First, it’s large, and when new employee types are added, it will grow. Second, it very clearly does more than one thing. Third, it violates the Single Responsibility Principle7 (SRP) because there is more than one reason for it to change. Fourth, it violates the Open-Closed Principle8 (OCP) because it must change whenever new types are added. But possibly the worst problem with this function is that there is an unlimited number of other functions that will have the same structure. For example, we could have

isPayDay(Employee e, Date date)

deliverPay(Employee e, Money pay)

The solution to this problem is to bury the switch statement in the basement of an ABSTRACT FACTORY, and never let anyone see it. The factory will use the switch statement to create appropriate instances of the derivatives of Employee, and the various functions, such as calculatePay, isPayday, and deliverPay, will be dispatched polymorphically through the Employee interface.

OTHER THINGS TO BE TAKEN CARE OF

Extract Try/Catch Block

Try/catch blocks are ugly in their own right. They confuse the structure of the code and
mix error processing with normal processing. So it is better to extract the bodies of the try
and catch blocks out into functions of their own.

For Example:

public void delete(Page page) 
{
 try 
 {
    deletePageAndAllReferences(page);
 }
 catch(Exception e)
 {
   logError(e);
 }
}

DRY (Don’t Repeat Yourself)

Look at the code below and figure out if we are breaking the DRY principle?

class CsvValidation
{
    public function validateProduct(array product)
    {
        if (product['color']==null) {
            throw new Exception('Import fail: the product attribute color is missing');
        }

        if (product['size']==null) {
            throw new Exception('Import fail: the product attribute size is missing');
        }

        if (product['type']==null) {
            throw new Exception('Import fail: the product attribute type is missing');
        }
    }
}

Implementing DRY Principle

class CsvValidation
{
    private productAttributes = [
        'color',
        'size',
        'type',
    ];

    public function validateProduct(array product)
    {
        foreach (attributes in productAttributes) {
            if ((product[attribute]==null)) {
                throw new Exception(sprintf($'Import fail: the product attribute {attribute} is missing'));
            }
        }
    }
}