Code smells are poor design and implementation choices that hinder comprehensibility and maintainability of code. Aniche, Bavota, Treude, Gerosa, and Van Deursen introduce a catalog of six code smells in Web applications that make use of the model-view-controller pattern.
Why it matters
Many studies have shown that code smells make code less maintainable and more prone to bugs. Some smells cause code to be changed more often due to violations of the single responsibility principle, which states that a class or module should have only one reason to change.
Most of these studies are based on a catalog of code smells that was originally defined by Martin Fowler and Kent Beck in Refactoring. The smells are generally applicable to any system written in an object-oriented manner, but overlook the role that a class or module may have in the system’s architecture.
This is why we have to study smells for specific architectures, like MVC. We must learn about their characteristics and impact, so that developers can understand how to avoid those smells and static analysis tools know how to recognise them.
How the study was conducted
The study makes use of surveys, interviews, and repository mining.
Creating the catalog
The authors started with a three-step data gathering process:
A simple survey that asked respondents to list good and bad practices for dealing with models, views, and controllers. The survey yielded 22 complete responses.
A more comprehensive survey that aimed to elicit good and bad practices for each of the five major MVC roles (controller, entity, service, component, and repository). This survey was completed by 14 respondents.
Unstructured interviews were held about good and bad practices for each of the five roles. The authors interviewed 17 professional developers, all of whom were familiar with the Spring MVC framework.
Two authors performed an open coding process to group good and bad practices into categories. Practices that were not specific to the MVC pattern were discarded.
The coding process resulted in a list of nine possible smells, which were presented to a core Spring MVC maintainer. Three of the smells were removed because they were specific to Spring and therefore not likely to affect users of other MVC frameworks.
Understanding code smells
To understand the characteristics and impact of code smells, the authors:
analysed 120 Spring MVC projects that are hosted on GitHub;
asked 21 Spring MVC developers to take part in a survey that assessed their perception of the six code smells;
looked for experts in development of MVC applications using frameworks other than Spring.
What discoveries were made
I’ll list the code smells first, and discuss their characteristics and impact afterwards.
Six MVC smells were identified.
A controller should provide cohesive operations and endpoints to clients, i.e. it .
Promiscuous controllers can be broken up into two or more classes until each controller is no longer promiscuous.
Flow control in controllers should be very simple, e.g. ideally a controller shouldn’t interpret input to determine what actions to take. Since a controller with a lot of flow control will be littered with method invocations, the authors argue that the number of non-framework methods that can be executed by a controller should never exceed 55.
Since business logic is supposed to be implemented in model layer, it has no place in controllers. Any business logic in a brain controller should be moved to an entity, component, or service class.
A service should contain business logic and/or handle business logic among domain classes, but never contain SQL queries.
Data access must always be handled by repositories instead.
Repositories are meant to handle anything related to data persistence, but should not contain complicated business logic or .
One could therefore argue that a brain repository is a class whose McCabe’s complexity exceeds 24 or SQL complexity exceeds 29. Complex logic and SQL queries should live in different methods. Logic that’s used by multiple repositories should be in a component.
Laborious repository method
Methods should have only one responsiblity and do one thing. For a repository method, this means that it should execute only one query.
Methods that execute multiple queries should be split, so that each method only executes one query. Methods can be private or public, depending on whether the persistence action makes sense on its own.
Each repository should only deal with a single entity, otherwise it loses its cohesion and becomes harder to maintain.
If a repository deals with multiple entities, each entity should get its own dedicated repository.
Characteristics and impact
The most common smell in the analysed dataset is the fat repository (20.5%), followed by the promiscuous controller (12.2%) and the brain controller (7.4%).
Brain controllers and laborious repository methods are often also affected by the traditional “complex class” code smell and in 59% of cases a brain controller is also a “God class”. The other smells do not appear to overlap at all with traditional code smells, which suggests that the smells in this catalog really are from a distinct category.
Impact on change- and defect-proneness
Analysis of Spring MVC projects shows that classes affected by MVC and traditional smells are significantly more prone to changes (almost 3 times more likely) and defects (2 times more likely).
These differences : classes affected by smells are still prone to changes, but are not more defect-prone.
There are also differences between MVC smells and traditional smells: the latter have a stronger negative impact on change- and defect-proneness.
Of the six MVC smells, the brain repository and meddling service have the strongest impact on change-proneness, while the meddling service is the only MVC smell that clearly results in more bug-fixing activities.
Perception by developers
Developers clearly perceive classes affected by MVC smells as problematic, particularly in the case of the meddling service, fat repository, and brain controller smells.
Some developers were able to correctly identify and define the smells without prior knowledge of the authors’ catalog. On the other hand, over half of all participants did not perceive classes affected by the laborious repository method as problematic.
Introduction and survival
Once an MVC smell is introduced in a system, it tends to survive for quite a long time. In general, there’s more than 50% chance that a smell will survive for longer than 500 days. Fat repositories even have an 80% chance of surviving more than 1,500 days. 69% of smells are never removed at all.
These smells : some smells already exist when the code artifact is first committed to the repository. For laborious repository methods this even happens 86.5% of the time!
Generalisability to other frameworks
Application built using frameworks that use the active record pattern don’t appear to suffer from meddling services and generally don’t use repositories, which would essentially eliminate three of the MVC smells. Instead, they’re more likely to suffer from fat models.
There are six MVC-specific code smells, which I have described in the “Code smells” section
Classes affected by an MVC smell are more prone to change than other classes, but traditional smells still have a stronger impact
A class affected by the meddling service smell is more prone to defects
MVC smells generally appear when an code artifact is initially created rather than through maintenance activities
MVC smells tend to survive for a very long time: 31% of all smell instances are never removed after their introduction