Published January 8, 2022
by Doug Klugh
Keep Functions from Straying
Avoid this code smell by encapsulating functionality with the data it changes most often. Feature Envy occurs when a function frequently interacts with data or functions in other modules. This violates the Law of Demeter by extending knowledge of other parts of the system. Apply the principle of Tell, Don't Ask to reduce query functions and encapsulate knowledge, while avoiding Anemic Domain Models. Consider the Common Closure Principle for functions that query other components and consider the Single Responsibility Principle for functions that query other objects within the same component. When needed, relocate whole functions by applying the Move Function pattern or split functions using the Extract Function pattern.
Refactoring requires many different skillsets. The first is being able to spot (or smell) bad code. These code smells are indicators (or symptoms) of underlying problems. Being able to identify these smells by name will not only help you to identify and categorize them, but also help to collaborate with your team. Identifying them by name will facilitate effective communication within your team and establish a common understanding of the underlying problem. This also helps to categorize, organize, and Manage Technical Debt.
Just about every common anti-pattern in software engineering has an associated smell. Experienced software engineers know many of them as they have inevitably come across them countless times throughout their careers. There is bad code everywhere — waiting to be refactored. If you don't know what a particular code smell is, search it. There are countless resources on the internet that explain code smells.
Following is a very short list containing just some of the common code smells (and anti-patterns) you are likely to come across:
Anemic Domain Models are objects that contain data with little or no associated functionality. Domain models should always be constructed by encapsulating functionality (business logic, calculations, validations, etc.) along with attributes and properties. They should not contain just data — that's the purpose of data structures. This will help to keep your service layer thin. The service layer should only be used to orchestrate behavior contained within domain objects.
Avoid writing tests that verify multiple assertions within a single test method. Numerous assertions make it difficult to determine which one caused a test to fail — inhibiting Defect Localization. This anti-pattern is often caused by Eager Tests that try to minimize the number of tests we need to write. Apply the Extract Function refactoring to tease apart these tests into single-condition tests so that the reasons for test failures are obvious.
Avoid CI Theatre by applying the fundamental practices of Continuous Integration (CI), along with supporting engineering principles, to achieve a mature DevOps Model for continuous software delivery. Precursors of CI Theatre include large user stories, difficult code merges, lengthy code reviews, poor test coverage, long-lived branches, siloed code ownership, broken builds for more than several minutes, and practicing Continuous Isolation by running CI on feature branches. CI is more about culture and mindset than any collection of tools.
Avoid this code smell by ensuring that your test methods do not verify too many test conditions. This often occurs when we try to minimize the number of tests we need to write. A unit test should verify only one condition by executing a single code path through the SUT. We achieve this by minimizing the Cyclomatic Complexity of the test and ensuring the SUT is isolated through Dependency Inversion. Acceptance tests should also be kept as short as possible. Eliminating this code smell will promote Defect Localization and minimize Test Overlap.
Avoid this code smell by encapsulating functionality with the data it changes most often. Feature Envy occurs when a function frequently interacts with data or functions in other modules. This violates the Law of Demeter by extending knowledge of other parts of the system. Apply the principle of Tell, Don't Ask to reduce query functions and encapsulate knowledge, while avoiding Anemic Domain Models. Consider the Common Closure Principle for functions that query other components and consider the Single Responsibility Principle for functions that query other objects within the same component. When needed, relocate whole functions by applying the Move Function pattern or split functions using the Extract Function pattern.
Avoid this design smell by building software that is highly modular, highly cohesive, and loosely coupled. A change to one part of your system should never break another part that is completely unrelated. High level policies (i.e. business rules) should never be impacted by changes to low level implementations (i.e. data persistence). Even related functionality should be decoupled enough to extend functionality without affecting related components. There are many engineering patterns and practices that facilitate flexibility, extensibility, adaptability, along with many other qualities that inhibit fragility.
Avoid this design smell by building software from which it is easy to extract and reuse internal components in new environments. This smell is often caused by dependencies that are tightly integrated with other parts of the system. Promote mobility by decoupling components from low-level implementations, such as data persistence, logging, user interfaces, etc. For example, business rules should be encapsulated within components to enable reuse across multiple systems.
Avoid one of the biggest technical debt patterns around — the monolith. Build modular systems by employing engineering practices to decouple your architecture and encapsulate functionality, while embracing modern cloud platforms to optimize reliability, resiliency, durability, scalability, and security for compute and storage systems. Software systems, especially cloud-native applications, should be built as a composite of small components and/or services that can be autonomously developed and independently deployed.
This code smell occurs when a test depends on mysterious external resources, of which the purpose is not well understood. This makes it difficult to understand the relationship between the test fixture and the expected behavior of the test.
Avoid building software that obscures its intent, contains duplication, or provides hooks in anticipation of future extensions. Instead, build an architecture that exposes its use cases, keep your code base as small as possible, and do not over-engineer your initial design (YAGNI). If your code is rigid, you are likely to add a lot of anticipatory design elements to minimize the amount of changes you will need to make later. Evolve your software from a Simple Design that is flexible, reliable, and easy to understand.
Avoid this test smell by writing tests from Simple Designs and refactor as needed to keep your test code clean. Good tests are understood at a glance and clarify the behavior they verify. Structure Matters as much for tests as it does production code. Apply design principles and patterns to enhance the structural quality of tests — as they are every bit as important as production code and should be given the same amount of attention. Behavioral quality can also be enhanced to ensure correctness of the tests by inhibiting errors from hiding within obscure code.
Avoid this design smell by building software that is flexible and easy to change. Rigidity is often observed when a small change forces a complete rebuild and redeploy. Small changes should be able to be built, tested, and deployed very quickly and independently of each other. Long build times are a symptom of high coupling. To promote flexibility, manage the dependencies between modules to ensure when one module changes, the others remain unaffected.
Build your software and your tests in ways that minimize test execution time. Common causes for long running tests include over-engineered test fixtures, asynchronous code, components with high latency, Test Overlap, and too many tests due to a tightly coupled architecture. This causes bottlenecks in Continuous Integration, inhibits rapid feedback facilitated by automated testing, and constrains frequent code merges that are required for Trunk-Based Development.
Think of switch statements as missed opportunities to be polymorphic. Instead of changing or extending a switch (or nested if-else statements), use Polymorphic Dispatch to invert the downstream dependencies. This is easily accomplished by replacing the argument of the switch with an abstract class containing a method that corresponds to the operation of the switch. Then simply create derived classes and implement that method for each of the cases. This will eliminate the need to recompile and redeploy that module (or possibly the entire application) any time a case is added or changed. This approach provides alignment with the Open/Closed Principle — greatly enhancing maintainability, adaptability, extensibility, and agility.
Avoid this anti-pattern by promoting collective code ownership to ensure knowledge is shared across the team and that everyone has the context they need to make good decisions. Every team member has equal ownership and accountability for the entire code base. Promote Separation of Concerns by building small, independent teams with individual code repositories and dedicated product backlogs. Conway's Law shows that independent teams build services with independent concerns.
Minimize the number of tests that depend on a particular piece of functionality such that each test condition is covered by exactly one test. Tests that verify the same functionality will usually fail at the same time and require the same maintenance when the functionality is modified. Give attention to tests that verify the same code in different ways as these may indicate different test conditions. Avoid Eager Tests that verify too many test conditions. Picking the right tests is essential in employing a risk-based approach to testing.
Avoid stringing method calls together on a single line of code to invoke methods within different objects. This type of call chain violates the Law of Demeter, while increasing complexity - inhibiting our ability to understand, maintain, and reuse the code. Instead of building a series of "train cars", apply the principle of Tell, Don't Ask to resolve all of these issues.
Avoid this design smell by building loosely coupled software systems through effective dependency management. A Viscose system is one in which development activities (such as building, testing, and deploying) are difficult to perform and take a long time to execute. When components span multiple layers of a system, it is often viscose because even the simplest change is costly to make. Apply the Component Design Principles to help manage the structure of large object-oriented systems.