16 deadly code smells — Part 1

Every programming language has one or more coding style guidelines. All developers know that but some of them don’t care. They practice bad programming habits for years without realizing the importance of the issues they cause.

As one of the hiring managers at Zuhlke Hong Kong, I’ve seen a lot of code smells during coding interviews. Here are the top 16 bad coding practices organized into 6 categories: coding style, dirty code, testing, error handling, code complexity, and optimization.

The first 8 code smells are covered in Part 1 and the rest in Part 2.

Coding styles

Ignoring the importance of coding styles usually indicates a lack of experience in creating software and reading other people’s code. It’s hard for inexperienced developers to understand the benefits of using a consistent coding style.

1. Inconsistent coding style

Consistency is one of the most dominant attitudes that good developers have. Being consistent is a result of logical thinking, which is a core element in software development. Every action that a developer chooses to take is because of some reasoning that the developer believes that it is true. Given the same environment, with the same input, we expect the same output. This is the same for the coding style.

Let’s compare the following snippets. Only one of them I consider as consistent.

Snippet 1:

if(document.ownerId == user.id) { doSomething(); }

Snippet 2:

if (document.ownerId == user.id){ doSomething(); }

Snippet 3:

if (document.ownerId == user.id) { doSomething(); }

Snippet 4:

if(document.ownerId== user.id) { doSomething(); }

Snippet 5:

if(document.ownerId==user.id){ doSomething(); }

Answer: Snippet 3

Compilers ignore most of the spaces we use in our code, so what are they for? Readability.

If you use a space between ) and { for better readability, then why wouldn’t you also use a space between if and ( as illustrated in snippet 1?

2. Inconsistent naming

Finding good names is not easy. It usually takes me a considerable amount of time. It’s part of the process of creating good software. Some people don’t agree and they name their variables or methods like this:

let value = 1; let curValue = 0; let previousVal = -1;

“Current” is represented in its short form in curValue, but this is not the case for “previous”. Instead, “value” is shortened as val in previousVal. The flow of the minds of these people is so random that no one can explain the logic behind such a naming style.

Dirty code

This is the exact opposite of clean code. When you see dirty code, you can feel its ugliness and want to look away immediately. You feel so uncomfortable that it makes it hard to breathe. It stinks and lingers for at least an hour.

3. Duplicate code

Duplicated code almost always indicates a failure in software design. So why does duplication ever exist? Some reasons I’ve heard from coding interviews are:

• They think they have no choice but the problem to be solved requires duplication

• They believe the easiest way to solve a certain problem is by duplication

• Some don’t even realize there is a duplication

All of the above are the result of incompetence in software design.

4. Bad naming

Who needs comments when names are self-explanatory and logic is clear?

Examples of bad names I saw in coding interviews:

• **retVal**: I cannot find an even less meaningful name for this. All values returned by functions are called return-value. So what is this particular retVal referring to?

• **tmp**: Almost every variable within a function scope is short-lived and temporary. So how temporary should one consider it as temporary? Even a temporary variable deserves a more meaningful name.

• Hungarian notation: Variable or function prefixing to indicate and check for types is no longer needed for all modern IDEs. Type prefixing makes it harder to change type and could lead to type inconsistency. It also reduces readability.

5. Dead code

There are two kinds of dead code:

• Unreachable code: Code that within a condition that can’t happen

• Commented code: Code that you are afraid to throw away

Unreachable code is less frequently seen during coding interviews because the problem to be solved isn’t that complicated.

On the other hand, commented code is a bigger issue that usually indicates a lack of understanding of source control.

If you ever want to comment code and think that you may need it at a later time, a better alternative is to use Git. For some IDEs, such as those made by IntelliJ, they provide local source control by default and the full history of your code is committed locally automatically. No one ever needs to comment on any code.

Testing

Every piece of sufficiently complicated code needs proper tests. Some people test manually, some test their code by logging, and good engineers write unit tests.

6. No tests

More than half of the candidates I’ve met in coding interviews didn’t write any unit tests. Some refused to write unit tests even after they were requested to do so. One simply cannot be sure their code works without any tests. Even if their code appears to work, there is no way to be sure it still works after refactoring.

7. No meaningful tests

Writing unit tests is the first step to becoming a better engineer. The next step is to write meaningful tests. Having tests that cover every single possible outcome is not meaningful and wastes time. Having tests that cover a few random cases is not any wiser.

Good tests should cover a few cases of happy paths and all edge cases. The ability to design good tests is one of the assessments for our technical interviews.

8. Console logging

It is also called Caveman debugging. It was a popular debugging technique decades ago and is the preferred way of debugging in extreme situations such as kernel programming where you don’t have any better alternatives.

Those candidates who print intermediate values to the console would argue that console logging is an easier and faster way to debug. It is almost certain that they didn’t know how to write unit tests and it would take longer time to complete the coding test during technical interviews.

Writing unit tests is as easy as printing to a console, if not easier, once you learn how to do it. Additionally, unit tests are needed for refactoring in which console logging is often removed because it reduces code readability.

If someone would consider printing anything to the console, that means there exists some code that is hard to understand. This is the case when unit tests are exactly for.

Continue with numbers 9 to 16 code smells in Part 2.

The important thing is to recognize code smells, especially in your own code. These are some of the very best books on how to write code that rocks:

• Clean Code: A Handbook of Agile Software Craftsmanship by Uncle Bob

• Code Complete: A Practical Handbook of Software Construction by Steve McConnell

• The Art of Readable Code: Simple and Practical Techniques for Writing Better Code by Dustin Boswell

• The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas