Qodana / Static Code Analysis Guide / What is a Code Smell?
Code smells are any parts of a program’s source code that show us our code might need refactoring or has some sort of deeper problem. In the same way, the smell of smoke can alert you to fire, so code smells can let you know it’s time to look for issues in your source code. As such, it isn’t a specific bug or a security alert for example, but it can indicate the presence of either or both. A code smell won’t stop software from working, but it might lead you to something that can.
American software engineer and creator of Extreme Programming, Kent Beck, first popularized the term in the late nineties, although it became increasingly popular as it appeared in more and more literature. Today, it’s a common term among many agile programmers and, in fact, in his Clean Code Series, Robert C. Martin (Uncle Bob) calls code smells a “value system for software craftsmanship”. This is just further proof that they shouldn’t be ignored, regardless of your team’s methodologies.
Code smells can be grouped by the nature of the underlying problems they indicate. Having categories helps you see a pattern in any potential issues and builds awareness of bad practices, making identifying these code smells in the code faster and easier. Let’s take a closer look.
Large solid sections of code are difficult to maintain and understand, and they often confuse developers. Divide your code into smaller pieces with clear names and a clear purpose.
Long methods are a sign that methods span over too many lines and take on too many responsibilities. The lengthier the method is, the more difficult it becomes to maintain and, even more importantly, understand. You can tell if a method is too long by looking out for red flags such as excessive interclass coupling or where you can’t immediately determine the function’s purpose. Refactoring to split the code into smaller, separate methods that are responsible for one task at a time helps to eliminate this code smell. Make sure these methods are clearly named to improve readability and make the code easy to follow.
Large classes often take on multiple responsibilities, have poor readability, and are difficult to reuse in other parts of the codebase. To fix this, you can extract parts of the class and break them down into separate classes with different responsibilities. As a result, your code will be much easier to read and reusable when needed.
Long parameter lists occur when a method is given too many parameters as an input.
This code smell should be addressed promptly because it makes the code more vulnerable to bugs and errors. Instead of using too many parameters, try creating a new object, which is a single object created from matching parameters. This helps to simplify the code and make it more maintainable.
Unnecessary complexity arises when code becomes overly convoluted, hard to read, and time-consuming to update or modify. This often leads to slower performance because many unnecessary tasks are being run. Developers can fix this issue by removing repeated code, refactoring with clear naming conventions, and using the available built-in functions for the programming language they’re using.
Long or complex conditional expressions occur when too many conditionals (if...else, true/false
) make the code difficult to understand and maintain. With large codebases, a small change might then need updates across multiple parts, taking up unnecessary time.
Testing becomes tricky because of multiple possible paths, which makes it complicated to cover all outcomes. The solution is to simplify – break down the conditionals into separate, clear parts and refactor the code to use functions to check complex data.
Spaghetti code refers to convoluted code that’s hard to untangle and understand. The problem is that it takes a great deal of time to figure out, and making changes or updates requires modifications in multiple places. You can spot spaghetti code by its unclear, complex structure and tight coupling. To fix this, refactor the code to simplify and condense it, focusing only on what is necessary for readability.
Dirtied code describes code that is cluttered and difficult to understand due to accumulated complexity. This can lead to it being challenging to extend, update, and maintain. It's also more likely to develop bugs or errors. To fix dirtied code, regularly clean up your code by removing redundant or unused parts. Refactor when needed to keep the codebase clean, readable, and efficient.
Repetitive code can be a breeding ground for errors that can't be readily fixed from one location, since any mistakes feature in multiple places in the codebase.
Duplicated code is one of the most common code smells, and occurs when the same code structure or logic is repeated. The trouble with duplicated code is that it makes updating your codebase challenging if the problematic code isn’t updated across all the instances in which it appears. This can lead to inconsistent code behaviors. To fix this, extract the duplicate code into a single function or method, and call it where needed.
Magic numbers code smells occur when a numeric value is hardcoded into the code without any clear meaning, making it difficult to understand its purpose. These numbers can easily lead to errors, as changes may require updating multiple instances of the same value. To fix this, define constants with meaningful names or use enums for related values. This will help you improve readability and facilitate maintenance.
Primitive obsession uses primitives instead of smaller, well-defined classes. This can become a problem because primitives are quite general, so you end up with unrelated or scattered logic. To resolve this, try replacing these primitives with objects by grouping related fields into their own defined classes. This improves code maintainability and organization.
Stringly typed code smells occur when a string literal is used instead of a more appropriate type (such as a class or data type). The issue with creating many strings is that it becomes easy to make mistakes, which can lead to initialization failing just because of a simple typo, for example. To remedy this, refactor the code to use value objects instead. As a general rule, try only using string literals when no other type makes sense.
Poor error handling is a problematic code smell because it can lead to poor user experience, such as freezing or crashing, and make the code difficult to debug. It also often leads to inconsistent code behavior and errors that are difficult to trace and fix. To rectify this issue, use try…catch blocks to catch and handle errors. This will improve the stability of the code.
Dictionaries and enums as objects come about when you use objects to define a set of named constants, which can lead to unclear intent and potential mistakes. Enums or dictionaries have the benefit of providing you with type checking and autocompletion, making your code safer and easier to maintain. Try replacing vague numbers or strings with enums that have clear, defined values. This reduces ambiguity and makes the code easier to understand and scale.
The high dependency between components makes them fragile – errors can be introduced with the slightest of code changes. To mitigate this risk, you can decouple classes or modules, separate concerns, and clarify purposes.
Tight coupling happens when components are overly dependent on each other to come up with one action or outcome. This can involve functions, classes, or modules that are tightly connected. It leads to a codebase that is hard to scale as you can’t scale or modify components independently. To resolve this, break down dependencies and make your components more flexible.
God objects refer to examples of when a single class takes on too many responsibilities, which can lead to excessive coupling and low cohesion. To address this, the class can be refactored into focused classes with single responsibilities. This will improve maintainability and readability as well as prevent performance issues down the line. Refactoring to meet the single responsibility principle and using clear naming conventions will enhance the overall quality of the code.
Shotgun surgery comes about when a single change to the codebase requires modification across multiple components, modules, or classes. This creates a risk that the code might break in several places as a result of a single change. The way to resolve this is to refactor in order to identify duplicate logic and extract this into a shared method, module, or component. Your code will be easier to maintain and less susceptible to bugs.
Feature envy occurs when a class’s method is overly dependent on another class. This often happens when a method in one class is too focused on accessing or modifying another class. To fix this, use the extract() function to break the function into smaller, more focused pieces. Then, apply the move() function to move the method to the most relevant class.
Difficult-to-test code happens when hidden dependencies or tightly coupled code make it hard to isolate different components for automated testing. Without proper test isolation, you may end up having to do manual testing, which is time-consuming and can lead to undetected issues. To remedy this issue, decouple dependencies and break down large methods or functions into clean, simple units.
Global variables can be accessed from any part of your code, which makes them useful at times, but you can run into problems later down the line if you overuse them. Excessive use exposes the code to side effects like bugs and unnecessary complexity, and makes it difficult to understand and work with. To address this, encapsulate variables inside methods, functions, or objects, limiting their global accessibility.
The distant or inaccessible class code smell occurs when a class is too obscure and difficult to access. This results in the code being hard to understand and maintain and also increases the risk of bugs. To remedy this code smell, make it more modular by using dependency injection. You want your dependencies to be logically linked to each other and therefore easier to manage. You can also organize the codebase and make classes clearer and more obviously related to each other.
Code with many comments can indicate a lack of clarity, which is why it’s important to try and write code that’s self-explanatory.
Adding comments to your code might seem like a good way to make your intentions clear, but it can become a problem when the code needs to be updated because the comments can quickly become outdated or redundant. The goal should be to write clear, readable code that speaks for itself, reducing the need for comments. Instead of explaining what your code does, focus on meaningful variable names and neat formatting.
Comments code smells happen when unnecessary comments clutter the code, even when the intent or function is already simple and clear. This can hurt readability and lead to outdated or missing information if the comments are not constantly updated. To avoid this code smell, before you comment, ask yourself if the explanation is absolutely necessary to understand the code. If the code speaks for itself, there’s no need to add a comment.
There are places in any codebase that need much more attention than a solution that works but “needs improvement”. Time is a precious resource that should be invested more efficiently.
Diminishing returns suggest that you may be spending time and effort on improving low-impact areas of code that do not hold much value. If you are refactoring, make sure you are doing it with a clear purpose in mind. If the code you are working with already reads well, there’s no need to further optimize it. Prioritize tasks that have a high impact and will make a bigger difference, and then measure the real benefits of your optimizations.
Make sure code gets reviewed before merging pull requests (PRs), and look out for code that takes a long time to review or large PRs because these could indicate that your team is expending effort cleaning up smelly code or updating large amounts of code just to be able to make the necessary changes.
One could argue that the best ways to avoid code smells are regular refactoring, continuous integration, and automating parts of your code reviews – and that’s where it pays to mitigate some of the human error and use a good tool to help you spot issues.
Qodana can help you identify potential issues, code smells and so much more. Once you commit changes to the branch, Qodana will do a background scan on the CI side and automatically submit the issue report to your IDE.
You’ll receive automated feedback on bugs, duplicated code, code smells, incompatible dependencies, and security vulnerabilities.
Get instant feedback in your IDE with automated reports from your CI pipeline.