Focus on the model
I read and summarise software engineering papers for fun, and today we’re having a look at Code smells for model-view-controller architectures (2018) by Aniche and others.
Why it matters
Code smells are poor design and implementation choices that hinder comprehensibility and maintainability of code.
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 researchers:
- 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.
Code smells
Six MVC smells were identified.
Promiscuous controller
A controller should provide cohesive operations and endpoints to clients, i.e. it should depend on a limited number of services (at most 3) and handle at most 10 routes (*).
(*) The values 3 and 10 were derived using a formula that can be found in the original article. Don’t think of these (and other numerical values in this section) as absolute thresholds; they’re more like rules of thumb.
Promiscuous controllers can be broken up into two or more classes until each controller is no longer promiscuous.
Brain controller
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 researchers 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.
Meddling service
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.
Brain repository
Repositories are meant to handle anything related to data persistence, but should not contain complicated business logic or complex queries (**).
(**) More specifically, queries that join multiple tables with complex filters, queries that are constructed dynamically, or objects that are manually assembled from query results.
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.
Fat repository
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 become smaller when artifact size is taken into consideration (***): classes affected by smells are still prone to changes, but are not more defect-prone.
(***) More code means more opportunities for bugs to appear, so this isn’t very surprising of course
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 researchers’ 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 are not always caused by code aging (****): 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!
(****) Do keep in mind that this analysis was done on open source projects. Closed-source projects may have different characteristics.
Generalisability to other frameworks
Most of the identified smells are generalisable to other frameworks (specifically VRaptor, Ruby on Rails, ASP.NET MVC, and Play!).
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.
Top comments (0)