Hello and welcome!
First of all, I have to say I highly respect Dan Abramov for his contributions to the JavaScript community. I think he is a strong influencer and thus, every time he speaks about something, it will affect a lot of people. When I recently came across his tweet referencing Goodbye, Clean Code, I couldn't help myself but react to it:
βοΈπ New on Overreacted: Goodbye, Clean Code overreacted.io/goodbye-clean-β¦21:31 PM - 11 Jan 2020
I believe that having multiple opinions/points of view on a specific subject is very important to understand it as a whole, so... Here are my takes on this matter!
TL;DR the real problem is not removing code duplication, it's changing the code of a colleague unbeknownst to them, and not questioning the abstractions chosen to solve the previous problems.
The definition of clean code really depends from one developer to the other. In Dan's article, it looks like clean code refers to applying the DRY principle: Don't Repeat Yourself. In my opinion, there are way more elements that compose the definition of "clean code", but let's focus solely on avoiding code duplication, as it appears to be the main subject here (this, and another issue regarding teamwork, but we'll talk about it as well).
The context
The story starts when a coworker implements a solution to a problem, but it involves a lot of repetitive lines of code. This code duplication bothers the author, so he refactores the code to avoid duplication and make it DRY. The new code is half the size of the previous one and has no repetitions.
In my opinion, the result is better: the logic (i.e. math formulas) can easily be unit-tested, and only once. In addition, the behaviors can be composed to generate new behaviors that fix the initial problem. There are also fewer lines of code, which means fewer places where nasty little bugs can hide.
Once the author was pretty happy with the refactoring, he committed the changes on the main branch, then went to bed. The next morning, he was told by his boss to revert the changes.
The phase
Now comes the second part of the article where I (partially) disagree with Dan.
It looks like he claims that writing clean code is "a phase" that has an end, which I disagree with. The definition of code "cleanliness" may differ through time, but it's still a good practice to adopt, no matter the years of experience one has. I don't believe that it has an end.
To me, the "phase" (if any) is the one that goes from "discovering the wonders of abstraction" as a junior developer, to "applying abstraction at every single line of code" as a more experienced developer. Although this is the red zone, it doesn't have to end there. Maybe that's what Dan meant in the first paragraphs of the "Itβs a phase" section, but the end of the article "Then let it go" suggests that developers should let clean code go enterily at the end of this phase, and that's where I disagree.
It's important to go through this phase to learn about abstractions and clean code, and it's even more important to know the right threshold of abstraction once this phase is over. First gather experience by over-abstracting things and seeing the consequences, then use this valuable experience to use the right abstraction, to write clean code.
There is a reference to The Wrong Abstraction article in there. I highly recommend you to read it if you haven't done it already. Essentially, it tells us that removing code duplication, by using abstractions, eventually leads to more complications in the future (e.g. when there are new requirements), because we don't dare remove the existing code. However, there is a very important part in this article that is often overlooked in my opinion:
Once you completely remove the old abstraction you can start anew, re-isolating duplication and re-extracting abstractions.
I feel like a lot of people that reference this article (for the sake of saying "hey duplication is actually good" maybe?) seem to forget this crucial part. It doesn't tell us that duplication is actually good. It tells us that it's fine to break/remove the abstraction and go back to duplicating the code temporarily when implementing new requirements. However, once the new requirements are handled, we have to think about a new abstraction (given the new state of the code) that will feel appropriate, and remove the code duplication.
The reasons why the refactoring was unsuccessful
The article continues with the enumeration of 2 reasons explaining why the refactoring was not appropriate.
Reason 1
The first reason is because he didnβt talk to the person that wrote the initial implementation. I completely agree on this one.
I don't believe in sabotage. When I write code, I believe that the solution I find is a good fit, given the context I am at that moment: my experience as a developer, my understanding of the code base and the requirements, the time granted to solve the problem. It might actually not be as "clean" as I think it is, because of multiple reasons such as code convention and best practices of the project, team knowledge about some paradigm or library I'm using, or lack of tests to name a few. But in my mind, I'm convinced I'm writing the best code I can think of, given the context.
I would feel bad if I spent time writing code to solve a problem, went to bed, then woke up seeing that it got almost enterily rewritten by someone else. I would think "Well, I guess it wasn't that good then... But why? Why did this other person think it was ugly? I spent some time writing this, and it was actually useless". It would destroy my motivation, and my self-esteem. I would be "scared" to write anything, or spend too much time on it, since it could be completely rewritten over night.
This is not a critic about the author, since he acknowledges that he shouldn't have done it as the first reason. This is an attempt to make people realize that this type of behavior can have deep consequences on the mental health and productivity of their coworkers. Please, if you work in a team, do not do this.
What can you do then? If the code written by someone else is bothering you for some reason, e.g. you don't think it's "clean", then I encourage you to do the following:
- Talk to your team by explaining what clean code is to you.
- If everyone agrees (or at least a vast majority), then write some documentation about code conventions and best practices.
- You will probably compromise, as some people will think that "this part of the definition is too extreme, too demanding". It's up to you to decide what to do, but my recommendation is that the team is greater than player: if the team thinks that your definition of clean code is too restrictive, then you should find a suitable tradeoff.
- Clean code won't be written magically by everyone as of the next day. A period of adaptation will take place. It could last weeks, or months. It's up to you to educate your teammates by leaving appropriate comments during code reviews, with examples and references to the documentation aforementioned.
Collaboration is key in a team working on the same project. Talk to them, teach them what clean code is, and why it should be written that way. Chances are you will learn things by listening to their points of view, and you will adapt your definition of clean code, for the better.
Reason 2
The second reason mentioned is one that bothers me. The author states that his refactoring would have made the new requirements difficult to implement, so in the end the original one with a bunch of duplication made it pretty easy to solve the new problems. The thing is, code changes over time, no matter the code. It changes because users have different needs and requirements, bugs are found, performance needs to be improved, or technologies must be updated/upgraded.
Code, whether it's duplicated or abstracted, will change.
Saying that an abstraction makes new requirements difficult to implement implies that the code written in the past cannot be questioned, challenged, changed, or adapted. Again, let me quote this part from the "The Wrong Abstraction" article:
Once you completely remove the old abstraction you can start anew, re-isolating duplication and re-extracting abstractions.
This means that the abstraction implemented with the refactoring to remove the code duplication could have been replaced with a better one, taking into account the new requirements. One does not have to choose between "code duplication + easy future implementations" and "abstraction + difficult future implementations". We can have "abstraction + easy future implementations", but for that, we need to be comfortable in undoing previous abstractions and adding temporary code duplication to come up with a new, better abstraction.
Should you write "dirty" code? As mentioned in the article, no.
Should you find your own definition of "clean" code? Yes, definitely.
Can this definition change depending on the people/project you are working with/on? Yes, definitely.
Should you be a clean code zealot? As mentioned as well, no. Nothing good ever happens in the extremes of any scale.
Should you let clean code go? No, never.
Maybe I am still in "The Phase" and I am simply not aware of it (yet?). I have been developing for 7 years professionally. I have known some moments of "clean code zealot", but I didn't stay there. These few moments made me a better developer and team player, because they made me realize that I was not alone working on these projects, and the team is greater than the player. I will probably encounter more of these moments in the future, but that's fine because I will grow a better developer. I will not let clean code go, because I'm convinced of its advantages.
A few months ago, I wrote about a way to solve problems while limiting the technical debt, which is introduced in a project because of a lack of clean code, in my opinion. I talk about clean code and how it can be used in a team there, feel free to have a look: A Method To Deliver Updates Quickly Without Compromising the Future.
Thank you for reading this far! I hope you enjoyed the ride. Feel free to share your opinion! :)
Cover photo by Kalle Schmitz on Unsplash.
Top comments (0)