Clean Code: A Few Key Rules to Follow

I don’t want to be hyperbolic, but I’m going to anyway. I think we can separate code into two simple categories: Bad code and clean code.

One type causes inefficiency, chaos, and wastes time. The other allows developers to jump in and out of projects effectively and with ease to build the cool, exciting things we all want to be a part of.

This article talks about how to detect bad code, avoid it, and some rules developers should follow so they don’t spend two hours trying to figure out what their colleague was doing with that function.

What is bad code and why is it a pain in the a%&?

Bad code causes a lot of distractions. On long term projects where many developers work together and perform several changes a day, bad code can creep up on your project real fast. That’s just the law of entropy.

This causes developers to waste time and energy navigating through functions, classes, files, and pouring over code trying to understand it.

Bad code is also highly interconnected — it arrives when you realize you have to make a minor functionality change, and then you have to modify a multitude of functions, classes, and files to accommodate that small change.

As time passes, the efficiency of the team decreases significantly. You could argue that clean code then, is a matter of professional survival.

Productivity Vs Time working on bad code

The merits of clean code

Working on a project where developers care about clean code makes it easy to understand its virtues; you can read the code from top to bottom in one go and clearly understand what it does. It makes your code manageable and maintainable in the long term, isolating changes and increasing the efficiency of your team.

You don’t need to apply all the rules all the time; the idea is to make you — as a developer — get into the habit of thinking critically and rationally about the code you are writing so that before your send your next PR, you review your code and can objectively ask yourself: Is this clean code?

Here are the some rules that will help you affirmatively answer that question:

1 — Naming

Everything in code hides a meaning behind its use; it has an intention and you must reflect this meaning using an appropriate name for it.

Meaningful names

Use meaningful and explanatory names and avoid one or two-letter, ambiguous names. For example, if in a function you return d, the meaning behind that function is hidden. Instead, return isClientSubscribed and avoid the misinformation. If the name includes half-truths or outdated information we are also adding confusion to the concept behind the name.

In this example we are calculating a state object to return inside a function. You should avoid names like objRet, etc. They don't add any semantic or meaningful context for that returned object.

function createStateObject(data) {
    var objRet = {};
    
    objRet.pageNumber = data.pageNumber;
    objRet.pageId = data.currentPage.id();
    objRet.postId = data.currentPost.id;
    return objRet;
}

The better idea is to use a name like stateObject

Similar names for similar concepts

Use similar names for similar concepts and implement meaningful variations for similar names in favor of autocompletion. Avoid adding noise by using variations like a1, a2, etc.

Example: Meaningful variations for similar names and concepts

Abbreviations and encoding

Avoid abbreviations or encoding. This forces the reader to learn that encoding and then perform a mental map for that concept every time they have to read, write, or look for it, adding a new layer of complexity to the code.

In the next example, the developer used an internal abbreviation and encoding to refer to a PushNotifications component in UI.

If you complete the name pn-layer__example using pushNotifications-layer__example in the name, it will be a lot more meaningful than:

.pn-layer__example {
  position: relative;
  top: 5px;
  margin: 5px;
}

Classes, methods, and functions

Classes are nouns. Methods and functions are verbs.

Functions must always do something that must be reflected in their name. A name like calculateAmount() is correct if that function just calculates that amount. If it does other calculations or sets an internal status, then we are misinforming the user and introducing side effects that will probably occur when other developers use that function. If this is the case, rename the function to calculateAmountAndSetStatus(). Or better, rewrite it into two separate functions (calculateAmount() and setStatus()) and introduce a better, higher logic using it.

Concepts

Always use one name per concept. Use get or fetch, controller, or manager... but always use the same name for the same concept.

Context

If you feel that you can’t find a better name, and it doesn’t reflect the full meaning for your code, you can add extra information inside the context for that name, and you can use that context to add meaningful distinctions to several names.

In the following example, there are 3 JavaScript classes with the same name, but we are adding context to the name (via their directory) so that the contents are explicit:

The first and the second one are adserver implementations, the third one is an implementation of Facebook embeds, and the last one is a social provider to get access to the Facebook API.

If we follow these simple rules, the names we give to elements of our code will be more easily identifiable, pronounceable, and searchable. If we try to find it in our code where the object Cp is used, it’s much more difficult to find than if we try to look for the uses of a class named ColumnPool.

Also, imagine we need to discuss that class with other developers. Talking about ColumnPool instead of Cp is much easier and more effective.

2 — Comments

In general, comments are excuses for bad code. If you feel the need to add a comment, you probably need to rethink and rewrite the code.

While comments can add basic information that the developer needs to understand the code, they can also add misinformation, confusion, and it will not be removed or modified if the code changes.

They also become read only lines exclusively. If you write a piece of code, you probably won’t be the next one modifying it. Also, the new developer will not modify the comments according to the new changes. This, in time, will be a deprecated comment and will add noise and confusion to the code.

Therefore, it’s critical that we can identify how to avoid comments, and when we should use them where appropriate.

Ineffective comments

Ineffective comments are the ones that add misinformation, are incorrect, or outdated. They can also be unnecessary or redundant. Attributions or logs are a good example. Developers should use the other mechanisms at their disposal to provide the necessary context their code needs through comments such as using Git, commenting on the commits, and adding annotations or code blame.

class UpperMenu {
    constructor(container, openHandlers, header) {
        this.upperMenuContainer = container;
        this.openHandlers = openHandlers;
        this.header = header;
        this.isInitialized = false;
        this.plugins = [];
        //initialize other properties
        this.isOpen = false;
        configureAnimations.call(this);
        configureLinks.call(this);
        configureAppInstallIcons.call(this);
     }

An example of a redundant comment

The above example is also an example of an outdated comment. It makes no sense to do insert it in the initializations of that constructor function. Looking into the history log, we see that it comes from an old version of the code that another developer didn’t remove. It might have made sense in the past, but is now just outdated and incorrect.

Good comments

Good comments are the ones that add basic and extra information like time calculations or warnings of the consequences in a temporal push when we are on call solving hot issues in production.

The next example adds basic, extra information to add readability to our code. It explains that the calculation means 24h in milliseconds.

Utils.setCookie('timeToLive', '300', 24 * 60 * 60 *1000); // 24h in millis

Tip: In these cases it’s better if we can remove the comment and we declare the value as a const with a descriptive name:

const DAY_IN_MILLIS = 24 * 60 * 60 *1000;
Utils.setCookie('timeToLive', '300', DAY_IN_MILLIS);

Sometimes we are also dependent on the technology or architecture that, just like our code, has bugs and misbehaves. Because of this we need to coexist with that and perform some kind of hacks and workarounds. In these cases, comments are needed to add that information and context. They should always include a brief explanation and point to a URL for a full explanation.

return {
    prepareColumnWidth: function prepareColumnWidth(viewNavigator) {
        var column = viewNavigator.column,
            containerSize = geContainerSize(column);
        // Due to a bug in iOS with position fixed:
        // http://stackoverflow.com/questions/26391059/new-fixed-position-bug-on-ios8
        column.style.width = containerSize.width + 'px';
        return containerSie;
     }
};

3 — Formatting

Readability affects maintainability in the long term. Proper formatting must be a team rule by convention. It sets a precedent for future changes and facilitates the quick impact of new developers joining the team and integrating themselves into the project seamlessly.

Currently, there are numerous ways to ensure and be conscientious of proper formatting. In the past, developers needed to think about it to write better code. Think back to programming on real VT terminals with 80 columns or text editors without really smooth horizontal scrolls. You always tried to write lines with less that 80 letters to avoid that kind of scroll.

Modern tools

More moderns tools are available to remond you of the rules when you don’t apply them. The lint or linter set of tools analyzes your source code to detect errors and suspicious code.

Setting a bunch of rules to manage the vertical and horizontal formatting is also a good strategy.

Rules

Set rules to:

• Account for the horizontal length of the lines
• Maintain the horizontal openness and density to be more readeable
• Add a new indentation per block
• Group by conceptual affinity.

In general, know the possibilities of your linters and set the necessary rules to add readability and maintainability to your code.

4 — Functions

Functions should be small. To be small, you must follow the One Thing Rule:

Functions must to do just one thing, and do it well.

If you find that you are doing several things in one function, then rethink and split it into functions with self explanatory names to better explain what it is doing. As always, use descriptive names. Look for a verb to explain what the function is doing.

Query — command separation

If a function is a query (get, set, is, kind of functions), it cannot be a command (a function that processes or calculates something). If you mix and try to find a proper name to describe the function's behaviour, then you will write a name with an and or an or in the function's name. This isn't exactly considered a best practice.

Let’s reintroduce the calculateAmountAndSetStatus() example that was presented in the Naming section. This name is a Query(set) and a Command(caluclate); a good example of what to avoid and when to refactor your code to split the function into two separate ones.

Side effects

In this regard, it’s important that we avoid side effects. Do just one thing, and avoid doing the undocumented things. If another developer uses your code without knowing that you are doing other operations inside it, you will most likely cause side effects including bugs and disfunctionalities.

Levels of abstraction

Avoid mixing levels of abstractions into one function. If you are operating with your ViewNavigator object, and you are doing Column operations in a higher level, calling methods like Column.left(); or Column.resize(); you can't access the DOM directly by doing querySelectors to get the status class of an element.

Encapsulate the details of implementation into a higher level of abstraction, to make functions more readable and to not create dependencies with a low level detail of implementation.

The next example can be refactored, because we are mixing levels of abstraction calling higher abstraction level functions (like the prepare or reloadImage functions) and accessing the low level details of implementation (classLists or registering events):

function imageLayoutChange (image, layout) {
    const onImageComplete = function() {
        this.classList.remove("imageLoad");
    };
    image.classList.add(layout);
    image.classList.add("imageLoad");
    image.prepare();
    image.reloadImage();
    Util.registerListener(image, 'load', onImageComplete, this);
}

For example, it can be refactored into this — a more readable code by just renaming and encapsulating it into auto self-explained functions:

function imageLayoutChange(image, layout) {
    const onImageComplete = function() {
        removeLayoutLoadingClass(image, layout);
    };
    addImageLayoutLoadingClass(image, layout);
    image.prepare();
    image.reloadImage();
    setImageOnLoadHandler(onImageComplete);
}

Parameters

Avoid functions with a big list of parameters. More parameters implies that more testing is needed. You can probably create a new data object to group all the parameters.

Null and error codes

Avoid returning null or error codes in a function, otherwise you will force the user of that function to check the return status before using it. Null and error codes also introduce a higher level of logic to manage error handling and can obscure the main logic of your code.

Repetition

Don’t repeat yourself. If you detect that you are writing very similar code over and over again, simply refactor your code. I know, you’re probably in a hurry to release your change or new feature into production, but this directly impacts the maintainability of your code in the long run. Every time you need to change that code, you will have to do it twice or even more.

Here we have common code inside these two methods, one to load and another one to unload media videos from our ViewNavigator Columns:

function unloadVideos() {
    var videoElements = videoSelector(),
        iframe;
if (videoElements.length) {
        for (var i=0; i<videoElements.length; i++) {
            iframe = videoElements[i].querySelector('iframe');
if (iframe) {
                iframe.setAttribute('data-src', iframe.src);
                iframe.src = 'about:blank';
            }
        }
    }
}
function loadVideos() {
    var videoElements = videoSelector(),
        iframe;
if (videoElements.length) {
        for (var i=0; i<videoElements.length; i++) {
            iframe = videoElements[i].querySelector('iframe');
if (iframe) {
                iframe.setAttribute('src', iframe.dataset.src);
                iframe.removeAttribute('data-src');
            }
        }
    }
}

And can be transformed into this, where we have the common code and we inject the method to use to load or unload videos:

function manageVideoMedia(method) {
    var videoElements = getVideoMediaElements(),
        iframe;
    
    if (videoElements.length) {
        for (var i = 0; i < videoElements.length; i++) {
            iframe = videoElements[i].querySelector('iframe');
            iframe && method(iframe);
        }
    }
}

5 — Classes

Classes are a higher level of code organization and just like functions, classes must be small.

Vertical Distance Rule

Developers should follow the vertical distance rule where methods and functions are grouped by vertical affinity on related concepts. This greatly enhances the readability of your code.

Single Responsibility Principle

The Single Responsibility Principle is where a class must only abstract one thing, have one responsibility, and just one reason to be changed in the future.

If a class has more than one responsability, then it will have a higher probability to be changed to add more functionality to that class. This breaks the OCP (Open Close Principle). A class must be opened for extension, but closed by modification. This means that you won’t want to modify the class every time that you extend the functionality.

Here is a classic example about writers and readers. In the first code example we see a function that reads from the keyboard device, and writes it to an output device, the printer.

So it’s highly connected with the details of the implementation on how to read and write to that concrete device.

void Copy () {
    int c;
    while((c = ReadKeyboard()) != EOF) {
        WriterPrinter(c);
    }
}

What will happen to this code if we need to write to another output device? At first glance, we can think about writing some code like this:

enum OutputDevice {printer, disk};
void Copy(outputDevice dev) {
    if(dev == printer) {
        WritePrinter(c);
    } else {
        WriteDisk(c);
    }
}

Now we are forking the code with the use of an if or a switch. This is not good because we're probably breaking the One Thing Rule (not in this case, but this is not a clean code).

To deal with this we can apply the DIP (Dependency Injection Pattern). DIP states that abstraction cannot depend on the details of implementation. The details of implementation must depend on abstractions. Lets see a better approach:

class Reader {
    public:
        virtual int Read() = 0;
};
class Writer {
    public:
        virtual int Write(char) = 0;
};
void copy(Reader &r, Writer &w) {
    int c;
  
    while ((c = r.Read()) != EOF) {
        w.Write(c);
    }
}

In this example, the Reader class no longer depends on the details of implementation of the current device. This respects the OCP: If we need to add a new device we will only be depending on the contracts of the Reader and Writer classes. So the details of implementation will be dependent on the abstractions. We will implement new Readers and Writers to accomplish that, avoiding the need to modify the original code over and over again.

Now take a look at your last PR. How clean was it?