This is a situation that happens all the time in teams developing code together. The first developer creates the abstraction, and the second developer feels the need to continue with it, and to refactor, despite the fact that it;s not really working. What Sandi does not talk about here is team dynamics, and communications. This is really a root cause of the problem.
The barrier for for the second programmer is cost of communication. It is far easier to continue a bad abstraction than to engage in discussion and perhaps arguments about the code the first programmer wrote. Her suggestion, to revert the abstraction and go back to duplication, will most certainly result in a long thread in the code review, or even a heated discussion between the two programmers. That's OK, but it assumes the team can handle this situation gracefully.
When a programmer spends time to come up with an abstraction, it often takes a lot of mental effort. The resulting code is something that the first programmer feels made everything better. They removed duplication, and they are proud of the new shiny abstraction. Simply reverting it is a big message that says "that was a bad idea", and is sure to bruise egos.
Perhaps the problem of wrong abstractions can be avoided in the first place when a team has a strong code review culture, and when developers pair more. In these teams, code ownership and shared responsibility lead to a lower barrier for making changes to abstractions, which are generally held on to more strongly.
In my experience, a codebase that suffers from wrong abstractions (and over-engineering in general) is also a sign that the team is not working well together. Typically one or more programmers are committing code without discussing it, due to bad team dynamics, physical distance, different POV on the project goals, or unhealthy "rockstar" mentalities.
I've never really thought about it this way (bad code quality as the product of bad team dynamics), but it makes perfect sense to me. I guess the way forward is to improve interpersonal relations.
In my experience code review is not a protection from this. Rather the opposite actually, since reviewers often identify very similar blocks of code that should be extracted to methods/classes. Those observations are not always correct.
We don't use abstractions merely to reduce duplication, and the removal of duplication is not the only, and not nearly the best, way of creating them. In my experience, abstractions arise naturally from thinking about the task to be done and the options available to do it. If an abstraction does not help in understanding a design and simplify the task of explaining it, it is probably not a good one.
Unfortunately, Fagan's rules for inspections try to rule out this sort of open-ended issue, and a code review is too late to be discussing it, anyway.
A developer's job is not to create abstractions; doing so is merely a technique - a very important, effective and general one, but not an end in itself, and when it is mistaken for one, that is how you get over-engineering.
If previous developer left the team, code not commented (or/and documented) and not known why abstraction was created, revert may by preferred solution to move forward.
This article creates a false dichotomy. The problem is not the result of programmer A's actions, but by B's insistence on forcing his changes into the existing pattern instead of refactoring. It is irrelevant whether the original abstraction arose from deduplication.
I do agree that pressing on instead of backtracking is a cause of many problems, but the solution is not simply to proliferate duplication.
I think the article agrees with you, but another point it makes implicitly is that programmer A is also on the hook for creating a brittle abstraction. The article is specifically warning against the temptation to deduplicate things for its own sake, without having in mind a good abstraction that describes the shared functionality implemented by the duplicated code.
You make a good point, though if A could not have anticipated the future changes, his actions may have been justified at the time. One justification that often applies to deduplication is to reduce the chance that future changes would miss one of the cases, but that only applies so long as the changes don't break the abstraction.
Avoiding brittleness is worthwhile up to a point, but overly-general abstractions often carry a cost in unnecessary complexity. From my own experience, I can think of three examples where generic programming, a container framework, and a DSL were used respectively, to create inordinately complex solutions having degrees of flexibility that would never be useful.
Yes!! Very true!! I would add that even if the introduced abstraction is NOT "wrong", adding more abstractions has its own cost in terms of readability, understandability, navigatibility, etc. of the code. Even if the abstraction is not "wrong", adding a new abstraction to remove 10 lines of duplicated code is not always a good tradeoff!
Abstracting out details so the "big picture" becomes clearer can make code more understandable. At the same time, more abstractions means more "layers", which a reader has to dig through to find out what is actually going on, making code less understandable. So there is a tension between allowing duplication, and removing it by adding a new abstraction, and which choice is better depends entirely on the specific situation.
Why is this tagged 2014? The article was only published yesterday. (August 26 2015)
It does reference a 2014 talk in the first paragraph, but the interesting parts about recovering from the wrong abstraction are all new (2015) content.
Why touch the abstracted method/function when it's perfectly being used by other parts of the code?
If there is a new requirement that will need you to change this method/function, that means that you need to copy it and make sure you name it to easily document whatever this new requirement is.
Now if this abstracted method/function has a lot of code that you don't want to be duplicated, then you might wanna refactor it with SRP as your guidance.
In addition to SRP, side-effect free methods/function will greatly help.
This is a situation that happens all the time in teams developing code together. The first developer creates the abstraction, and the second developer feels the need to continue with it, and to refactor, despite the fact that it;s not really working. What Sandi does not talk about here is team dynamics, and communications. This is really a root cause of the problem.
The barrier for for the second programmer is cost of communication. It is far easier to continue a bad abstraction than to engage in discussion and perhaps arguments about the code the first programmer wrote. Her suggestion, to revert the abstraction and go back to duplication, will most certainly result in a long thread in the code review, or even a heated discussion between the two programmers. That's OK, but it assumes the team can handle this situation gracefully.
When a programmer spends time to come up with an abstraction, it often takes a lot of mental effort. The resulting code is something that the first programmer feels made everything better. They removed duplication, and they are proud of the new shiny abstraction. Simply reverting it is a big message that says "that was a bad idea", and is sure to bruise egos.
Perhaps the problem of wrong abstractions can be avoided in the first place when a team has a strong code review culture, and when developers pair more. In these teams, code ownership and shared responsibility lead to a lower barrier for making changes to abstractions, which are generally held on to more strongly.
In my experience, a codebase that suffers from wrong abstractions (and over-engineering in general) is also a sign that the team is not working well together. Typically one or more programmers are committing code without discussing it, due to bad team dynamics, physical distance, different POV on the project goals, or unhealthy "rockstar" mentalities.
I've never really thought about it this way (bad code quality as the product of bad team dynamics), but it makes perfect sense to me. I guess the way forward is to improve interpersonal relations.
In my experience code review is not a protection from this. Rather the opposite actually, since reviewers often identify very similar blocks of code that should be extracted to methods/classes. Those observations are not always correct.
We don't use abstractions merely to reduce duplication, and the removal of duplication is not the only, and not nearly the best, way of creating them. In my experience, abstractions arise naturally from thinking about the task to be done and the options available to do it. If an abstraction does not help in understanding a design and simplify the task of explaining it, it is probably not a good one.
Unfortunately, Fagan's rules for inspections try to rule out this sort of open-ended issue, and a code review is too late to be discussing it, anyway.
A developer's job is not to create abstractions; doing so is merely a technique - a very important, effective and general one, but not an end in itself, and when it is mistaken for one, that is how you get over-engineering.
If previous developer left the team, code not commented (or/and documented) and not known why abstraction was created, revert may by preferred solution to move forward.
This article creates a false dichotomy. The problem is not the result of programmer A's actions, but by B's insistence on forcing his changes into the existing pattern instead of refactoring. It is irrelevant whether the original abstraction arose from deduplication.
I do agree that pressing on instead of backtracking is a cause of many problems, but the solution is not simply to proliferate duplication.
I think the article agrees with you, but another point it makes implicitly is that programmer A is also on the hook for creating a brittle abstraction. The article is specifically warning against the temptation to deduplicate things for its own sake, without having in mind a good abstraction that describes the shared functionality implemented by the duplicated code.
You make a good point, though if A could not have anticipated the future changes, his actions may have been justified at the time. One justification that often applies to deduplication is to reduce the chance that future changes would miss one of the cases, but that only applies so long as the changes don't break the abstraction.
Avoiding brittleness is worthwhile up to a point, but overly-general abstractions often carry a cost in unnecessary complexity. From my own experience, I can think of three examples where generic programming, a container framework, and a DSL were used respectively, to create inordinately complex solutions having degrees of flexibility that would never be useful.
Yes!! Very true!! I would add that even if the introduced abstraction is NOT "wrong", adding more abstractions has its own cost in terms of readability, understandability, navigatibility, etc. of the code. Even if the abstraction is not "wrong", adding a new abstraction to remove 10 lines of duplicated code is not always a good tradeoff!
Abstracting out details so the "big picture" becomes clearer can make code more understandable. At the same time, more abstractions means more "layers", which a reader has to dig through to find out what is actually going on, making code less understandable. So there is a tension between allowing duplication, and removing it by adding a new abstraction, and which choice is better depends entirely on the specific situation.
Meta:
Why is this tagged 2014? The article was only published yesterday. (August 26 2015) It does reference a 2014 talk in the first paragraph, but the interesting parts about recovering from the wrong abstraction are all new (2015) content.
Why touch the abstracted method/function when it's perfectly being used by other parts of the code?
If there is a new requirement that will need you to change this method/function, that means that you need to copy it and make sure you name it to easily document whatever this new requirement is.
Now if this abstracted method/function has a lot of code that you don't want to be duplicated, then you might wanna refactor it with SRP as your guidance.
In addition to SRP, side-effect free methods/function will greatly help.
I hope I made sense here.