What is code smell?

A code smell is an in­dic­a­tion of poor code quality. If code smells are present, you get the feeling that something is wrong when viewing the code. We’ll show you the known code smells and explain how to get rid of them.

A code smell is a char­ac­ter­ist­ic of code that causes ex­per­i­enced pro­gram­mers to feel that the code isn’t clean. Having a good `nose´ for code smells is roughly com­par­able to the intuition of a master craftsman. If he sees a tangle of cables or badly laid wires, he im­me­di­ately knows that something is wrong. Looking at the source code is the only way to notice the dif­fer­ence.

The well-known British pro­gram­mer Martin Fowler points to his colleague Kent Beck as the creator of the term. On Fowler’s blog, he provides the following defin­i­tion:

Therefore, a code smell indicates a systemic problem. Either the code was written by a pro­gram­mer with in­suf­fi­cient com­pet­ence or by con­stantly changing pro­gram­mers. In the latter case, the fluc­tu­at­ing levels of com­pet­ence, knowledge of the codebase and fa­mili­ar­ity with guidelines and standards leads to poor code quality. The code starts to stink.

A code smell is not to be confused with selective de­fi­cien­cies, which are to be expected in every codebase. First, a code smell is only an in­dic­a­tion. If a pro­gram­mer notices a code smell, further ex­am­in­a­tion is necessary to determine whether there’s actually a systemic problem.

If it becomes apparent that the code smells because it’s bad, there are different ap­proaches to cleaning up the code. Often these are re­fact­or­ing ap­proaches, which are designed to improve the structure of the code while main­tain­ing func­tion­al­ity. However, depending on the size and com­plex­ity of the code, it may be im­possible to remove code smells. Then only a `re-write´, i.e., starting from scratch, will help.

Code is a medium that exists at different levels of ab­strac­tion. Let’s think of an ap­plic­a­tion. This consists of a multitude of in­di­vidu­al items, all of which are defined in code like variables, functions, classes, modules. To consider the different code smells, we dis­tin­guish between the ab­strac­tion levels:

1. General code smells
2. Code smells on a func­tion­al level
3. Code smells on a class level
4. Code smells on an ap­plic­a­tion level

General code smells can be found on every level of an ap­plic­a­tion. In most cases, they are not clear errors, but rather patterns that have a negative impact. A rule of thumb in pro­gram­ming states that code should be written primarily for the reader. In­ex­per­i­enced pro­gram­mers often write code that delivers the desired result but is in­com­pre­hens­ible.

Naming in­di­vidu­al code con­structs well, such as variables and functions, is a fine art. Un­for­tu­nately, there are often mean­ing­less, non­sensic­al or even con­tra­dict­ory names in code. The prime example is variable names con­sist­ing of only one letter: x, i, n, etc. Since these names don’t give any context, sense and purpose are not apparent.

It’s better to use ex­press­ive names. Then the code reads like natural language and it’s easier to follow the program flow and detect logical in­con­sist­en­cies.

Clean code looks like it was written as an in­teg­rated whole. You can im­me­di­ately see that the code was created based on certain rules. If this reg­u­lar­ity is missing, it’s a code smell.

Variation in naming suggests that several people wrote parts of the code in­de­pend­ently of each other. If it was a team, it suggests there was obviously no uniform spe­cific­a­tion.

Some languages such as C++, Java and JavaS­cript allow you to declare a variable without spe­cify­ing an initial value. The variable exists from the de­clar­a­tion but is not defined. This may result in subtle bugs. This code smell is par­tic­u­larly fatal in care­lessly typed languages because without an initial defin­i­tion, it’s not re­cog­nis­able which type is expected for the variable.

In­ex­per­i­enced pro­gram­mers often make this mistake. To compare a variable with a specific value, they enter the value directly. This is also referred to as `hard-coded values´. Hard-coded values are prob­lem­at­ic if they appear in several places in the program. This is because the in­di­vidu­al copies tend to mutate in­de­pend­ently over time.

It’s better to define a constant for the value. This es­tab­lishes a `single source of truth´ (SSOT), i.e., any code that needs the value accesses the same constant defined in a single place.

A magic number is a special case of hard-coded values. Let’s imagine that our code works with a value limited to 24 bits. 24 bits cor­res­pond to 16,777,216 possible values or the numbers from 0 to 16,777,215.

Un­for­tu­nately, a comment is missing so it’s not clear how the value came about later. A magic number has been created. The lack of knowledge about the origin of the value increases the risk that it will be changed ac­ci­dent­ally:

We can spare ourselves these sources of error. We include the defin­i­tion of the value as an ex­pres­sion in the as­sign­ment of the constant. This way, we make the idea behind the value explicit. Op­tion­ally, we use a sub­sequent assert statement to document the actual value in the code:

In most languages, it’s possible to switch code con­structs ar­bit­rar­ily deeply. Un­for­tu­nately, this also increases the com­plex­ity, which makes it more difficult to see through the code when reading it. A par­tic­u­larly common code smell is deeply nested control state­ments such as loops and branches.

There are several ap­proaches to resolving the nesting. Here we use the Boolean AND operator to place the two con­di­tions within one `if´ statement:

Another approach, which works with any number of con­di­tions, uses `guard clauses´. We break out of the function as soon as one of the necessary con­di­tions is not fulfilled:

In most languages, functions are the basic unit of execution of code. There are smaller pieces of code, such as variables and ex­pres­sions, but these stand alone. Writing clean functions requires some ex­per­i­ence. We’ll show you a few of the most common code smells on a func­tion­al level.

Functions receive input values as arguments. In many cases, only certain values or value ranges are valid. It’s the re­spons­ib­il­ity of the pro­gram­mer to check input values for validity and handle ex­cep­tions ac­cord­ingly.

Scopes are a fun­da­ment­al feature of most pro­gram­ming languages. They determine which names are defined at which positions in the code. Sub­or­din­ate scopes inherit from the scopes above them. If an ex­tern­ally defined variable is accessed within a function, this is a code smell. This is because the value may have changed between the defin­i­tion of the variable and the function call:

Ideally, you should only access values within functions that have been presented as arguments. To avoid having to present the same value over and over again, default para­met­ers are used:

Object-oriented pro­gram­ming (OOP) can help to increase code reuse and reduce com­plex­ity. Un­for­tu­nately, the odds of making mistakes in class design are high, which later become no­tice­able as code smell.

One of the most common code smells on the class level is the ‘god object’ or the cor­res­pond­ing ‘god class’. A god object combines all kinds of func­tion­al­ity that don’t actually belong together and violates the principle of ‘sep­ar­a­tion of concerns’.

Code smells are often en­countered in con­nec­tion with the in­her­it­ance hierarchy. Sometimes code is un­ne­ces­sar­ily dis­trib­uted across multiple levels of in­her­it­ance. The mistake is also often made when in­her­it­ance is used instead of com­pos­i­tion as the primary approach to composing objects.

The un­in­ten­ded use of ‘data classes’ is also con­sidered a code smell. These are classes that don’t implement their own behaviour. If no methods are defined other than generic getters and setters, you should consider using a simpler data structure such as a Dict or a Struct.

It’s every pro­gram­mer’s nightmare: you’re assigned to work on an existing ap­plic­a­tion, and a first look at the code reveals that the entire ap­plic­a­tion is in a single, huge file. It’s unclear how the in­di­vidu­al code com­pon­ents are related. Like a bowl of pasta, everything is haywire in the ‘spaghetti code’.

Ab­strac­tions are missing, there’s no sub­di­vi­sion into classes and, at best, only a few functions. Instead, there are all sorts of code du­plic­a­tions, which result in subtle bugs. Fur­ther­more, there’s a lot of use of global variables.

The use of global variables is a par­tic­u­larly pervasive code smell. Because global variables can po­ten­tially be changed from anywhere in the code, you open the door to hard-to-fix errors. When debugging, you are forced to look for errors every­where.

Another ap­plic­a­tion-specific code smell is the lack of sep­ar­a­tion of concerns. Es­pe­cially in PHP projects, it’s easy to mix markup, func­tion­al­ity and present­a­tion in one file. This makes code reuse and re­fact­or­ing difficult. It is better to strive for a clean sep­ar­a­tion of concerns, e.g., by using the ‘Model-View-Con­trol­ler’ (MVC) pattern.

Another ap­plic­a­tion-specific code smell is the lack of sep­ar­a­tion of concerns. Es­pe­cially in PHP projects, it’s easy to mix markup, func­tion­al­ity and present­a­tion in one file. This makes code reuse and re­fact­or­ing difficult. It is better to strive for a clean sep­ar­a­tion of concerns, e.g., by using the “Model-View-Con­trol­ler” (MVC) pattern.

In principle, it’s prefer­able to con­tinu­ously keep code clean. The best way to do this is to follow the pattern:

1. Prototype: create draft
2. Test: test the func­tion­al­ity
3. Refactor: clean up the code
4. Ship: deliver code in pro­duc­tion en­vir­on­ment

Un­for­tu­nately, the third point is often skipped, es­pe­cially when the decision is made by a manager with no coding ex­per­i­ence. The reasoning is: the code works so why keep putting effort into it? Over a longer period of time, the code starts to stink and technical debt ac­cu­mu­lates.

If an existing codebase contains code smells, it’s not possible to start from scratch. The task then is to clean up the code. The best approach is to take an in­cre­ment­al approach and first delineate areas within the codebase that can be cleaned up re­l­at­ively easily. Separate the working or im­prov­able parts from the ‘dark corners’ that are better left alone.

Sep­ar­at­ing the less stinky code areas from the more stinky ones reduces com­plex­ity. This makes it easier to test code and perform debugging steps. Automated code review tools that detect code smells and offer sug­ges­tions or tips for cleaning up can help.

Re­fact­or­ing ap­proaches are effective in removing code smells. Func­tion­al­ity is en­cap­su­lated in functions or existing functions are split. Removing unused sections of code and improving the naming of the code makes the codebase leaner. The golden rules ‘Don’t repeat yourself’ (DRY) and ‘You ain’t gonna need it’ (YAGNI) help as guidelines.