Does your project have an enormous module called something-manager or something-controller that grew beyond any possible limits and became completely unmaintainable? How many do you have?
Usually, code smells are the results of deliberate actions:
We had a tight deadline and agreed to ship overcomplicated and unreadable code. We never prioritized writing tests, and now we little confidence in changing old logic.
Large modules are a different story. They do not happen overnight, and I'm yet to see a person suggesting to put a ton of random methods to one pile and call this pile GlobalManager or Utils. It is never a choice, but rather the result of many good intentions: small enough to attract attention, but still dangerous to cause harm.
Evolution of one module
Let me start from an oversimplified fictional example.
Imagine that you work on a piece of software for college students and their teachers.
First of all, we need to allow teachers to see all assignments that a particular student has. Let's write a module for it!
This is what we need to do:
- Check that a user is a teacher and hence has necessary permissions
- Get all we need from the database
- Format the result and show it to the user
A work in progress, but nothing terrible is happening so far.
Time goes, and teachers request another feature: they also want to see which books their students have. Sounds like a weird request, but we can live with that.
For this feature, we can reuse several functions we wrote before (getRole, formatResult and showContent). All we need to do is to add this small change:
At some point later, we get another request, but it is not about teachers anymore — students also want to keep track of their books to know what they need to return to the library.
This one is even easier to implement:
tiny function that reuses previously implemented capabilities — easy choice, easy code review.
But now students have one more request: they need more information about their books, i.e. the date when it should be returned. Something that formatResult cannot do. Luckily, we are programmers and can make them happy:
Job is done!
The last piece of news for this example: teachers decided that the strange case about seeing students' books was redundant, and there is no real situation for which it would be needed.
Who does not like removing code, huh?
We must be proud because we maximized code reuse and iteratively supported new use cases with minimal code changes. All commits were concise and sensible, at least at first glance.
However, it is worth checking the module once again — not only the changes but full content:
Two different types of content for two different types of users. Not much overlap.
What's much worse is that this module is prone to unlimited expansion. Every small change related to students, teachers, books or assignments is justifiable until at some time we wake up with a module so big that the only thing left to do is teach it how to make coffee. Fortunately, the water is already boiling.
Why it happens
Several factors are contributing to this process.
We, developers, love reusing code. It saves us a lot of effort — can you imagine the world where we had to write everything from scratch?
At the same time, we do not like changing old code. Sometimes it is hard, sometimes it is unsafe, sometimes it just makes us think.
Code that we can reuse — investment.
Code that we have to change — legacy.
That leads to the thinking pattern which I call Reckless Reuse:
How to avoid
To prevent Reckless Reuse, keep in mind the original purpose of a module you intend to change:
In our example, we could move the functions getRole and showContent outside of the module that serves teachers, so we still can reuse them from other places of the application without having to grow the teachers' module.
Unfortunately, it is never enough to just do it yourself – all collaborators should adopt the same practices. There are a couple of tricks that can help you with that:
- Invest in proper naming.
Make sure the names of your files reflect their purposes. If naming is hard, it may mean that you try to call something that should never exist.
- Write comments about the goals of your modules.
If the name alone is not explanatory, consider adding a paragraph or two about which set of problems a given module is supposed to solve.
It should help you to notice when modules start doing something beyond their initial scope. Even when a change is small and reasonable, it will raise questions when somebody adds the method trackAttendance into the module book-data-provider.