I’ve heard this argument against deduplication many times before. Wrong abstraction is not a solution to duplicate code. It’s a different problem. If developers are not able to perform the simplest of abstractions, the problem is more serious.
Far more often than writing an obvious and easy to see wrong abstraction on the spot, the usual case in reality is that someone reasonably factors multiple parts of the code to depend on a single piece of reused code, and the multiple parts have very subtle and hard to recognize difference in their goals. Often these differences don’t manifest until more code accumulates and the goals begin to drift further apart. By the time it becomes clear, the dependency is harder to undo.
The problem is more serious precisely because developers generally are able to perform the simplest of abstractions and more.
I think correct abstractions reduce dependency issues.
If an abstraction simplifies the code when it is introduced, but turns out to be wrong in hindsight, then this is a problem of software evolution rather than wrong abstraction. In such a case, I would argue that abstracted code is better than duplicate code, because detecting that the duplicate code instances are the same is more difficult than finding all instances of the abstraction and correcting them.
We can’t debate or say anything useful about whether an abstraction is better than duplication without looking at specific cases; I would not presume to claim abstraction is better than duplication under any generic rule whatsoever. It simply depends on the code in question. That said, I don’t really understand what you mean about detecting duplicate instances being harder, because the premise of the abstraction being wrong is that you don’t want to find duplicate instances. If the abstraction becomes more wrong over time, then it doesn’t matter which task is harder, one of the two you mentioned would be going the wrong way. If the abstraction is wrong, then there’s by definition only one of them, no?
Definitely agreed. By software evolution, I was referring to your mentioning “goals drifting apart”. Code is always dynamic, constantly maintained, but goals don't always drift apart. When they do, it's because of unexpected circumstances. Doing a right abstraction for unexpected cases is almost not possible by definition. (It is, but in that case one falls into the more dangerous trap of premature abstraction.)
> We can’t debate or say anything useful about whether an abstraction is better than duplication without looking at specific cases
Agreed again. I'm aware that I'm overgeneralizing at the cost of neglecting any nuance. Once, I encountered nested loops iterating over a data structure and performing simple operations. I was abstracting these loops into a static method. Unfortunately, not all loops were exact copies. Some loops defined new variables while some reused the existing ones from surrounding code, some had different loop bounds while some performed the same functionality by breaking the loop inside an if. Some loops did a different job, but looked the same.
I had to be sure that each code piece was doing the same thing by manually inspecting and testing it (there were no automatic tests and code wasn't structured well for testing purposes). If these nested loops were abstracted in the first place, there wouldn't be any doubt whether they were doing the same thing or were slightly different. All of them would be the same function call. When the goals drifted apart, it would be easy to identify the points of evolution.
Thanks for your replies. They broadened my perspective.
Read this quote somewhere:
Make the code DRY, but not so DRY it chafes.
I'm not sure how I feel specifically about pair programming, but I do think that even "local" code changes should be accompanied by at least some architectural discussion with other developers who work on that part of the system. Usually the discussion can be as simple as "does XYZ design seem like a good idea? yeah, go ahead". But in my opinion it's important to encourage, incorporate, and expect collaboration as part of the basic workflow.
Perhaps this is also part of the problem with code reviews. Reviewing code is kind of difficult. But it's a lot easier if you've already discussed the design beforehand with the person whose code you're reviewing, so at least you already know why they did what they did, and you aren't going to be surprised, and then have to spend time writing up your disagreement and entering a back-and-forth process that sucks up time.
But I think you’re right - I think what you’re saying is code reviews should start before the change is being proposed for commit - it should be an architecturally collaborative process at some level. Code should be guided by reviewers in a positive feedback loop rather than left to critique at the last second. Reviewing code well is difficult, and also important to take seriously.
I somewhat agree, but not entirely, that the craft of programming is to make abstractions. There is some truth to that, but your statement implies that coding is only about making abstractions and nothing else. I also see programming as a means to an end, and I’m suggesting the end is usually more important than the means. It is possible to walk into existing code and reduce the amount of abstraction, and it is possible to make new features in existing software without adding any abstractions.
I’ve been writing software for a long time and watched quite a few people over-engineer their abstractions and cause real problems. I think there’s a human tendency to try to meet larger needs than we have at the moment, after all software is about automating and scaling. There is a reason why most sizeable software projects globally have been late and/or over budget. We are taught in CS school how to make abstractions, and we are not ever taught how and when to avoid them. Learning the art and craft of software includes how to see when abstracting something will leave things worse than you found them.
My tendency to watch out for over-abstraction has led me to fail to abstract things when I should. I have definitely made the mistake of staying too specific and allowing duplication to linger longer than it should. In a couple of cases it’s cost me weeks of time to fix. But, despite that, I still think erring away from abstraction, when it’s a real choice, is often the right call. On the flip side, I’ve witnessed over-engineering cost whole teams more than one or two years, and millions of dollars.
20 years ago I got hired to on CNC machines.
The torch on/off code was 50 lines long and cut and pasted in hundreds of places. It blew their minds when I explained what a function was. TorchOn()/TorchOff() completely blew their minds. This was for a highly successful product.