[foz]

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.

[mannykannot]

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.

[yxhuvud]

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.

[madawan]

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.

[citrin_ru]

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.