Cyclic Dependencies

Description: A cyclic dependency happens when a component (namely, A) depends on another component (namely, B) to perform its duties, which in turn depends on it (i.e., A). The same behavior can be extended to classes, functions, methods, and interfaces. The number of dependencies within the code is referred to as coupling.

Rationale: If a cyclic dependency happens when the components involved need to be modified, used, tested, extended, or used together, this is generally not a wanted behavior. Common cause of this type of smell is an improper design of responsibilities. A more subtle underlying issue is when a reference to self (this in Java) is passed between different components creating such dependency. Last, but not least, more complex systems might be nonobvious to mentally organize in terms of dependencies, hence resulting in unneeded introduction of flaws.

Violated principles: Readability, reusability, extensibility, testability, and maintainability.

Also known as: Circular dependency.

Variants: Circular imports, cyclic hierarchy.

Detection strategy: The simple case presented in the description could be spotted by simply carefully reading the code. However, cycles of dependencies can span across several components before returning to the starting point, hence closing the loop. A better approach in this case is to take a look at the class diagram. Inspect it to see if it behaves like a directed acyclic graph (DAG). If not, fix the cycles.

The following snippets show an example of cyclic imports. However, remember that the concept applies to classes and functions as well.

As from the preceding snippet, a cyclic dependency exists since module A relies on module B, which in turn relies on module A (import statements).

Feature Density

Description: This defect happens when a component implements more than a single functionality. Specifically, we refer to feature density when such components have a lot of dependencies that are not clearly structured. Those dependencies are such that the calling component performs some duties (i.e., logical functionalities) that should be performed by the callee. This might be the case, for example, of an object depending on multiple other objects to perform its duties. However, instead of having structured dependencies, it uses and builds on top of them eventually performing functions that should have been implemented by the dependencies.

Rationale: This smell breaks the single responsibility principle. Hence, it comes with all the inherited flaws. Given multiple—eventually independent—functionalities into the same component, it increases the chances of adding dependencies from many more other pieces of code as it gets extended further. Furthermore, changes to the affected component will impact all the dependent abstractions, hence impacting on maintainability.

Violated principles: Maintainability and single responsibility principle.

Also known as: Feature envy.^3,4

Detection strategy: For manual code reviews, check that Single responsibility principle is ensured. Each class, method, or function should have a single logical goal to be achieved.

Suggested refactoring: The solution to this smell is simple—keep logic around a single responsibility and cut the fat out. Move out methods that clearly need to be elsewhere. If several methods need to be factored out, it might be the case of having them in their own little class.

As you can see from the code, too much is achieved into the same class: doing coffee, managing user’s information, and—if it was not enough—customizing the coffee type based on learned preferences. This is enough to break the single responsibility principle . However, also pay particular attention to the gather_user_preferences method. The coffee machine should not access to user information and figure on its own what the preferences for the specific user are.

Unstable Dependency

Description: It happens if a component depends on a less stable one. This applies to any type of component (e.g., classes, methods), scenario (e.g., APIs, backend software), and type of defect in the code (e.g., design, technical debt, security).

Rationale: When a component depends on less stable ones, it will be easier to break the code.

Violated principles: Maintainability.

Detection strategy: Some tools and metrics are still under development or tuning. For example, a formula has been proposed to express to which degree a dependency can be considered unstable.⁵ The main concern expressed by the referenced work is that a package with a small number of bad dependencies may not be a smell. Hence, they use thresholds to signal this type of smell. However, more tuning of these thresholds might be needed.

Suggested refactoring: This smell is not always easy to fix. However, especially for production-ready code, try to find a more stable version of libraries used.

Figure 4-1 visually shows a relationship between a stable component and unstable one. As an example, the stable component is a clean coded component (e.g., a class) that imports another component (e.g., another class) which suffers from feature density.

As a consequence, how clean the stable component really is depends on the feature dense one, resulting in a theoretically stable component which indirectly is as much stable as the unstable one.

Mashed Components

Description: As the opposite of the feature density defect, a mashed component is one that should logically be a single functionality, while scattered on different places on the code.

Rationale: Scattering code which should be logically in the same abstraction has similar impact on the number of dependencies like the feature density smell. Indeed, to obtain the overall wanted behavior, multiple components need to be involved into the computation process. This has a severe impact on all the major principles of well-designed software. Also known as scattered functionality smell, this can be defined—in other words—as multiple components that head to achieving the same high-level objective.

Violated principles: Readability, maintainability, reusability, and testability.

Also known as: Scattered functionality.^6,7

Detection strategy: To detect this smell, take a look at diagrams (e.g., class diagrams) and inspect for highly coupled behavior. It might be the case of a mashed component smell.

Suggested refactoring: Rethink the provided abstractions and refactor the code accordingly. Remove any redundant code.

Finding the right trade-off between the two, hence having a well put together design, is not always obvious, and the only way to find it is, other than following the red flags and resolution techniques depicted in this chapter, to really think what make sense in terms of abstractions in the real world. Once you can clearly state responsibilities for each and every component, think about the problems around the interactions they will have and pick communication patterns accordingly.

Basic functionalities that could be part of a single component are scattered across more than one functionality. A simple refactoring in this case would be to allow FirstUserManager to print user information on its own.

Ambiguous Interfaces

Description: APIs should be extensible and flexible, not unclear or ambiguous. Ambiguity , as will be described in the following, can happen for several reasons.

Rationale: A fairly common example of an ambiguous interface⁸ is when a component offers from a single entry point. This is, for example, the case where a class provides a single public method that allows for actually accessing the class behavior. In real-world scenario, it is like imagining a website that needs to offer login, logout, and password retrieval functionality, and all these features are modeled with a single general interface. When this smell arises, a review of the actual code is needed in order to have an understanding of what the affected interface is meant for. Another common case of ambiguity is when multiple (i.e., more than one) signatures are too similar (e.g., same method name but with slightly different parameters). What can happen is that under certain scenarios, both of them might be applied, causing ambiguity in which one should be used.

Violated principles: Readability, maintainability, and reusability.

Detection strategy: Scan the code to figure out where interactions between components in the system are not clearly modeled.

Suggested refactoring: Rethink the provided interfaces and refactor the code accordingly. Disclaimer: the sooner the properly designed interfaces, the better. Neglecting to properly design interfaces might cause integration issues later on when this smell is found and changes are needed.

Consider the obvious case in which both name and surname are strings. In such conditions, both methods would apply.

As you can see, this code is cleaner , than the previous one due to the specificity of the signatures. Everything that is not of type (str, str) will be mapped to the first method with input type (object, object).

Mesh Components

Description: This is the case where components , are heavily coupled, with a lot of dependencies and oftentimes without clear and well-defined patterns.

Rationale: This smell might be caused by the mashed components smell; however, it is not always the case. It is a more general smell and the root cause might simply be a lack of abstraction and design patterns.

Violated principles: Maintainability, testability, readability, and reusability.

Also known as: Dense structure.

Detection strategy: Similar to the mashed components, to detect this smell, take a look at diagrams (e.g., class diagrams) and inspect for highly coupled behavior.

Suggested refactoring: As almost all the design issues, rethink the provided abstractions and refactor the code accordingly as necessary.

A simple example of this smell is shown in Figure 4-2.

As you can see from the example , there is no clear structure on how the components interact with each other. Some simple fixes (i.e., adding patterns and structure) include adding an authentication manager as well as a more structured way to interact with the database (e.g., a database manager).

The example shown above is fairly simple and presents only single direction dependencies. However, scenarios in industrial code can definitely be more complex and convoluted than this one.

What happens to the previous example if also another dependency is present from person details to the user?

For example, the user has personal details modeled separately (hence the dependency from user to person details), but the person details class also interacts with the user to access to authentication data (dependency from person details to user).

This last scenario is not only an indicator of lack of proper abstractions but also introduces, because of it, more smelly code, in this case a cyclic dependency.

First Lady Components

Description: This kind of component will do all the possible work by itself. It reaches such a growth that it becomes really expensive to maintain. The first lady component puts the feature density smell to its extreme. However, while a feature dense component only attempts to build functionalities that are not part of its duties and still relying partially on other components, a first lady simply does not have dependencies to components that should abstract separate behavior. It simply implements them all.

Rationale: This type of components is a conglomerate of independent behavior. It is similar to the feature density smell, but it is exponential in the lack of abstraction and of the negative impacts it has. As an immediate consequence of implementing multiple independent behaviors, the design of components that depend on a single logical functionality implemented by the first lady also becomes unclean, cascading to a heavily coupled overall design. Indeed, a single lady introduces possibly unneeded dependencies and well as a single point of failure.

Violated principles: Readability, maintainability, reusability, single responsibility principle, and testability.

Also known as: God component.^9,10

Detection strategy: Easily detect and inspect classes, modules, and functions. Those with a big number of lines of code (LOC) might be good candidates for this type of smell. It is also as simple as reading package name, class name, and the methods implemented in such a class. Simply reading them helps in spotting any behavior that does not belong to the given abstraction.

Suggested refactoring: Oftentimes this smell happens due to a lack of abstraction. Once you detect the affected components, rethinking the problem and properly defining behavior help in removing flaws.

A simple example of single lady is the scenario where there is a class that models the behavior of a user . This class, however, also implements general logging capabilities within the system. As an immediate effect, every other component that needs logging capability will have to depend on the user abstraction to access to logging.

As a cascading issue, what can also happen is that a developer reading the code can quickly, by reading packages and class names, assume that the system does not provide logging capabilities, hence providing an alternative implementation of logging. This not only ends in very unclean code, but it causes also time to be wasted to potentially reinvent the wheel.

That’s Not My Responsibility Component or Bossy Component

Description: At the opposite end of the first lady, this component likes to delegate stuff that it doesn’t want to deal with to other components. This component is not nice to have in code and oftentimes it twists the knife in the mesh components issue.

Rationale: This component also breaks the single responsibility principle—single means one, not none. Hence, it inherits all the flaws of mashed component smell, multiplied by ten.

Violated principles: Readability, maintainability, reusability, and testability.

Detection strategy: Inspect heavily coupled components. Is the caller only delegating logic to other components?

Suggested refactoring: Rethink the functionalities and refactor accordingly. Check if this abstraction is really needed.

The code is self-explanatory. You might just be curious why I used static methods. Well, it looks more authoritative, isn’t it?

The core issue of this smell is not that it has dependencies per se: we reuse behavior from other components when needed. Otherwise, no code in the world would be clean or even written. Instead, the key issue is when we introduce an abstraction that simply gathers together dependencies without adding value on its own.

At the end of the day, detecting and fixing this smell is really about asking yourself if the abstraction is really needed and has a purpose (no more than one, but a single one).

Summary

Even great clean code will start to smell over time. As you add functionalities and complexities, and as the programmers work on the code changes, the defects will build up and the smell will start to develop. That’s normal and that’s why healthy code requires incremental improvements and reviews to stay that way.

It is like going to the gym. You work hard and finally achieve the glorious six pack abs you’ve dreamed about. You are the happiest person on earth. How long do you think that happiness will last if you suddenly stop training and watching what you eat? Your dream abs will go away in no time.

So please, smell your code and do it often.

In the next chapter, we will introduce how to have a more comprehensive view at the overall structure of the code by means of looking at software architectures.

Further Reading

Smelling the code is for sure not a brand new topic, while often neglected. If you have to pick just one other book to read on the topic, go for Martin Fowler’s Patterns of Enterprise Application Architecture—an always current reference for design wisdom. Another good source for design smells is Refactoring for Software Design Smells: Managing Technical Debt by Girish Ganesh Samarthyam and Tushar Sharma.

Code Review Checklist