Maintainable Code - Good and Bad

While reviewing the code, sometimes nitpick comments are added for small-small improvements. And most of the time because of the lack of visibility it might make sense to ignore these suggestions. But believe it or not, these things slowly add up and over time, you end up with a lot of unmaintainable code and no time to correct your past mistakes.

There is plenty of code out in the open-source software world, but it is hard to tell where to start. Beginners get quickly overwhelmed with the complexity of professional code in real-life projects. And how do you know if a piece of source code is actually of high quality? Even if it is, how can someone with only a few months of programming experience distinguish it from a flawed hack?

Of course, if you already know more advanced books on code quality, readability, maintainability, and clean code, such as A Philosophy of Software Design and Clean Code, then you’ve already come a long way. Nevertheless, you can still find something new here and there.

In this post, we are going to cover things that are easy to get used to and best for the long term when applied. To demonstrate the good and bad practices we’ll use code examples. Comparing good code to bad code is helpful when you’re trying to figure out how to write better code. Here for demonstrations, we will be using JavaScript but the patterns can be applied to any language that you use. The goal of this post is to discuss the techniques that help you to write code that’s clean and to the point — the code that others will read with pleasure and reuse.

Before we start it’s worth discussing what is refactoring. Because 60% of the discussion of this post is about improvement through refactoring. Martin Fowler describes refactoring as—

Refactoring (noun): a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior.

Refactoring (verb): to restructure software by applying a series of refactorings without changing its observable behavior.

Let’s start our discussion to understand how to write code in the best possible way. By following these techniques you can cut clutter in your code.

1. Improving Conditions

Let’s start our discussion with conditions.

1.1. Avoid Unnecessary Comparisons

You should always avoid unnecessary comparisons, take following example-

if (isEligible(user) == true) {
  allotParking(user);
} else {
  throw new NotAllowedError("Not eligible for parking.");
}

In the above case, the if expression doesn’t require explicitly checking it with true. We can simply go ahead and make the following changes to improve this.

if (isEligible(user)) {
  allotParking(user);
} else {
  throw new NotAllowedError("Not eligible for parking.");
}

Here we have removed the explicit check with boolean value because we know the nature of the method.

1.2. Ensure Code Symmetry

Symmetry in code is valuable, and often subtle. Code symmetry helps to explain the intention of the code. Take a look at the following example.

// Bad
async function authorize(user: User) {
  if (!user || !user.isActive()) {
    logUnauthorizedAccess();
  } else if (user.isAdmin()) {
    await grantReadAccess(user);
    await grantWriteAccess(user);
  } else if (user.isModerator()) {
    await grantReadAccess(user);
  }
}

// Good
async function authorize(user: User) {
  if (!user || !user.isActive()) {
    logUnauthorizedAccess();
    return;
  }

  if (user.isAdmin()) {
    await grantReadAccess(user);
    await grantWriteAccess(user);
  } else if (user.isModerator()) {
    await grantReadAccess(user);
  }
}

Note the difference, now it’s clear for the method that the first if block is the guard clause that we have separated out and the core part of the method is symmetric now.

1.3. Avoid Negations

We sometimes use negations in conditional expression, but it might be easy to miss and can lead to debugging problems. Looking at the negation symbol ! we may get confused with the variable names starting with english letters(I, i, l). When the program is small or the codebase is small you might not realise this, but when you have to go through long diffs or codebase for debugging purposes, it might create problems. Consider the following example.

// Bad
if (!isAllowed()) {
  logUnauthorizedAccess();
} else {
  displayList();
}

// Good
if (isAllowed()) {
  displayList();
} else {
  logUnauthorizedAccess();
}

Here just by inverting the condition, we are able to get rid of negation.

1.4. Always Use Braces

We often write conditional statements without applying braces, this is a dangerous habit. Sometimes while refactoring or in regular code change, we might add a new line and you will endup finding no reason for it. Or if few statements should be grouped together, we might end up leaving few out of the block to be considered and might result in loopholes. These type of mistakes are not a syntax error that your compiler or interpreter will detect.

// Bad
if (isEligible(user))
  allotParking(user);
else
  throw new NotAllowedError("Not eligible for parking.");

// Good
if (isEligible(user)) {
  allotParking(user);
} else {
  throw new NotAllowedError("Not eligible for parking.");
}

1.5. Return Boolean Expressions Directly

When returning a boolean value, if you make use of if statements, it is easy to convert these into a single expression that returns the boolean value directly. Consider the following example.

// Bad
function isAllowed(age) {
  if (age < 30) {
    return true;
  } else {
    return false;
  }
}

// Good
function isAllowed(age) {
  return age < 30;
}

This solution was easy to follow because we knew that the method will always return boolean.

2. Improve Code Readability

All the sections in this post target to make the code more readable, but specifically in this section we are going to discuss a few things that are directly related to how specific portions of the code can be improved for understanding the complex logic of your application.

2.1. Use Proper Naming Conventions

How you and your team follows naming conventions can make a big difference in understanding what your code is doing.

2.1.1. Avoid Short Terms

When in code you use single letter names or abbreviations to declare resources like methods, local variables or class names; it makes it harder to understand the purpose of the resource. This may lead to confusion among contributors as well as for you in future.

2.1.2. Avoid Meaningless Terms

You should avoid meaningless terms and use the proper name which should lead to no confusion. Let’s assume you are trying to declare a new variable in a method, use the term which conveys the purpose. When writing a method that does something, use verbs.

2.1.3. Use Domain Terminology

The more domain terminology you will use, the better it will be to understand the code. If you’re working on some of the large projects, you may need to be consistent about naming the resources. For example, an app like Uber may need to name variables like driver, passenger rather than just user in the context of booking or reserving the cab, whereas in user management service may use the variable names like user.

2.2. Group with New Lines

Whitespace is crucial to clean readable code. A blank line (or two) helps visually separate out logical blocks of code. Consider the following example.

// Good
async function authorize(user: User) {
  if (!user || !user.isActive()) {
    logUnauthorizedAccess();
    return;
  }

  if (user.isAdmin()) {
    await grantReadAccess(user);
    await grantWriteAccess(user);
  } else if (user.isModerator()) {
    await grantReadAccess(user);
  }
}

Compared to above, try scanning the following code.

// Bad
async function authorize(user: User) {
  if (!user || !user.isActive()) {
    logUnauthorizedAccess();
    return;
  }
  if (user.isAdmin()) {
    await grantReadAccess(user);
    await grantWriteAccess(user);
  } else if (user.isModerator()) {
    await grantReadAccess(user);
  }
}

This is a small chunk of code, but when the line of the code will increase you will have a hard time understanding the logic of the code.

2.3. Replace Magic Numbers with Constants/Enums

Sometimes we directly use the numbers or the strings which can be either created as constant or enums. It’s always good to declare the constants at one place and use it across the program. If your constant is being used across multiple files, prefer creating a constant file and reuse the values whenever needed. This way, you solve two problems.

• In case if change to value is required, you just need to update at one place.
• Avoiding mistakes when you repeat the values in your code.

2.4. Favor Language’s Core API Over DIY

Sometime if you don’t know the language’s API or the libraries that are already included in your codebase. You might write some solutions from scratch. This is the root for many problems but most importantly for introducing inconsistent code. You should know the language’s API that you are using. You should also read the codebase if you are a new contributor. It’s easy to introduce DIY code, but this is not helping the project code in terms of maintainability. Tomorrow, if you need to make a change in these core DIY methods, you might miss making changes in other places. There is just one way to deal with this, know your language and know the codebase that you’re working on.

There are times when you need to favor third-party or custom solutions. For example in PHP native methods are much slower, JavaScript’s native Promise has poor performance. But whenever you go for a custom solution or third-party solutions, have strong reasons for doing so.

3. Improve Functions

Functions if not written properly can lead to few basic problems, even though it can handle the requirements. For example if the function is too large, it may lead to long-method code smell. If the function has too many branches or loops, it will increase cyclomatic complexity. Let’s look at a few rules of thumb for writing methods.

3.1. Split Method with Boolean Parameters, Optional Parameters

Be it any resource, classes or functions, it should always follow the single-responsibility principle. In functions specially, if you violate this principle, it may lead to many problems. Few of them are.

• Reusable code will be part of the method and when you may want to use the logic in some other place, you can’t do that.
• Testing these functions will be very tricky, because a function doing too many things will require you to cover all cases just in one method.

To avoid these problems, try to look for patterns on how you can break down large methods into small, reusable functions. For example, few if statements will be well suited to be created as boolean functions, loops inside a function might give you a pattern to see if the block can be moved to a process function.

3.2. Favor Immutable Over Mutable State

Mutable state works as follows.

• If two or more parties can change the same data (variables, objects, etc.).
• And if their lifetimes overlap.
• Then there is a risk of one party’s modifications preventing other parties from working correctly.

Note that this definition applies to function calls, cooperative multitasking (e.g., async functions in JavaScript), etc. The risks are similar in each case.

4. Improve Error Handling

How you handle errors has a lot to do with how stable your application is in worst case situations. Few things you should always consider.

4.1. Fail Fast

Systems should not fail, applications should not crash. That’s what we all want. However, sometimes it fails, and crashes, and we all try hard to prevent that from happening. Yet, there is a principle in software development that goes exactly the opposite ways: Fail-fast. Why? Because fail-fast makes bugs and failures appear sooner, and the end result is as follows.

• Bugs are earlier to detect, easier to reproduce and faster to fix.
• It’s faster to stabilize softwares.
• Fewer bugs and defects will go into production, thus leading to higher-quality and more production-ready software.
• The cost of failures and bugs are reduced.
• The longer it takes for a bug to appear on the surface, the longer it takes to fix and the greater it costs.

4.2. Always Catch Most Specific Errors

When you catch most specific errors, you avoid silently ignoring unknown cases. This is a really quick approach to find any logical or system problems early on. It gives the same advantage as fail-fast. Consider the following example.

// Bad
async function fetchProductById(id) {
  try {
    return await Product.fetch({ id });
  } catch (fetchErr) {
    return null;
  }
}

Now let’s look at the improved code.

// Good
async function fetchProductById(id) {
  try {
    return await Product.fetch({ id });
  } catch (fetchErr) {
    if (fetchErr.code === NOT_FOUND_CODE) {
      return null;
    }
    throw fetchErr;
  }
}

Notice the difference in catch block, in improved code it returns null only if the error code matches with NOT_FOUND_CODE. Which prevents it from silently passing when an unknown error occurs.

4.3. Explain Empty Catch

When gracefully handling cases, it’s important to write a comment in catch block why it’s not thrown back. At the time of implementation or based on some product requirement you may assume that it’s a well-known case but remember, the code that you write today can be managed by someone who is not aware of the context and might change something which can result in breaking older apps.

5. Improve Unit Tests

Unless you are writing unit tests, your code will end up hard to maintain and hard to fix. Well-written, thorough unit tests are just a big win all around. Let’s discuss a few points on improving the unit tests.

5.1. Structure Tests Into GWT

When it comes to unit testing, GWT stands for Given-When-Then. It is a general pattern for writing individual tests to make them more readable and useful.

First, you provide the setup. In this step, you set things up to be tested. You set variables, fields, and properties to enable the test to be run, as well as define the expected result.

Then you ask — that is, you call the method that you are testing.

Finally, you assert— call the testing framework to verify that the result of your call is what was expected. Follow the GWT principle, and your test will be clear and easy to read.

5.2. Favor Standalone Tests

This is probably the baseline rule to follow when it comes to unit tests. Any resource(classes or function) should be tested in isolation. They should not depend on anything other than mocks and stubs.

5.3. Cover the Edge Cases

The core target of writing unit tests is discovering edge cases and as soon as you find new cases you keep adding the test cases.

6. Improve Code Commentary

Let’s discuss how we can improve comments in our codebase.

6.1. Remove Commented-Out Code

Code gets commented because it was used earlier but currently not in use. But let’s discuss why few programmers want to keep commented-out code. They keep it because in future they may need to use the same code again. But think carefully, do we need to keep the commented code always? Can’t we use git to see history of our code or get the removed code from commits?

We should always remove the commented out code because there is no point in keeping it there. They just increase lines of code with no reason.

6.2. Replace Comments with Constants

There might be few comments added just to explain what constant the variable is going to hold. For example consider the following code.

// Bad
async function fetchProductById(id) {
  try {
    return await Product.fetch({ id });
  } catch (fetchErr) {
    // 2200 is code when the record is not found
    if (fetchErr.code === 2200) {
      return null;
    }
    throw fetchErr;
  }
}

Let’s improve the above code.

// Good
const NOT_FOUND_CODE = 2200;

async function fetchProductById(id) {
  try {
    return await Product.fetch({ id });
  } catch (fetchErr) {
    if (fetchErr.code === NOT_FOUND_CODE) {
      return null;
    }
    throw fetchErr;
  }
}

Here we don’t need comments to explain the error code. Another advantage we may get with this is that we can reuse the constant variables.

6.3. Replace Comments with Utility Methods

The same pattern can be applied to comments that are written for expressions that are trying to do something in a method. For example, converting temperature, rounding the number, and so on. These comments can be removed if we convert the lines to utility methods with proper names.

6.4. Document Implementation Decisions

The comments should be a rationale which captures “Why did I write this code in this particular way at this particular time?”. The reality is that code is usually never ‘finished’, code tends to evolve constantly, it tends to be constrained by real-world issues (deadlines, customer demands, budget, etc). so it will always have warts and imperfections which are beyond the developer’s control.

Your comments are the best place to document those things - avoid writing long essays, just stick to the minimum amount of information which might warn future developers away from making mistakes, or even just let them know that you’ve already considered the benefits/limitations of your solution (or even if you’ve found potential issues with someone else’s existing code and haven’t been able to fix it).

6.5. Structure comments for Methods

When we name the methods appropriately, our problem of the purpose of the method is solved. But still there can be a lot of confusion in the argument types, possible options, and optional arguments. If a developer has to go through the method definition to understand these things then it’s a waste of time because then he will have to also understand the code till the very end of the method definition. If method comments are written properly it helps in removing these confusions.

/**
 * This is a function.
 *
 * @param {string} n - A string param
 * @return {string} A good string
 *
 * @example
 *
 *     foo('hello')
 */
function foo(n) {
  return n
}

7. Final Touch

Let’s look at a few final points to improve code for better maintainability.

7.1. Use Static Code Analysis Tools

Can we ever imagine sitting back and manually reading each line of code to find flaws? To ease our work, several type of static analysis tools are available in the market which helps to analyze the code during the development and detect fatal defects early in the SDLC phase. Such defects can be eliminated before the code is actually pushed for functional QA. As we have already learned, that defect found later is always expensive to fix. You can use tools like SonarQube, ESLint or any-other tools available depending on the language you use.

7.2. Speed Up Your Program

Strive for performance. You don’t know later at some point where the code can be used. If you don’t pay attention to improving performance the end user will suffer because of this. For example, see if in your program if you are doing processing sequentially then can it be done in parallel? Or if you’re making duplicate network calls. You should always look for possible performance improvements in your code.

7.3. Automate Software Deployment Process

To increase the efficiency of software development, automate things as much you can. Consider the following flow.

• The developer makes the code changes, runs the tests to verify that everything works as expected.
• Next, the developer commits the code and pushes it to the remote feature branch.
• That triggers the CI pipeline, which can have multiple stages like unit tests, integration tests, build images. Each of these stages runs one after another. If at any stage failure is detected, the pipeline stops, and the failure is reported automatically.
• If the build is failed, set up some rules. Like pull-requests shouldn’t be merged. After the build is a success, we will have a feature build image ready which can be used to deploy the services in staging.
• QA finishes the tests on staging, then the code is merged to the master branch. That again runs the CI pipeline. But with the master branch, this also has one more stage, to trigger the deployment.

When we look at the flow mentioned above, it has fixed many issues that generally we face. With this, we have made our repository a single source of truth for all our deployment and test coverage.

Conclusion

Maintainability is inversely proportional to the amount of time it takes a developer to make a change and the risk that change will break something. Improving readability, coupling, or consistency all contribute to maintainability because it won’t take as long to make any given change. Maintainability is easier to recognize by its absence, like when something you thought should take an hour ends up taking a week.

I hope these techniques will help you to be good at finding maintainability problems when structuring your code. If you have any thoughts, please post them in comments.