3 ms·
I think the author got the wrong message from this situation. Yes, the refactor is bad, but I think they misidentified WHY it is bad. > Firstly, I didn’t talk
by kerkeslager 7y ago
I think the author got the wrong message from this situation. Yes, the refactor is bad, but I think they misidentified WHY it is bad.
> Firstly, I didn’t talk to the person who wrote it. I rewrote the code and checked it in without their input. Even if it was an improvement (which I don’t believe anymore), this is a terrible way to go about it. A healthy engineering team is constantly building trust. Rewriting your teammate’s code without a discussion is a huge blow to your ability to effectively collaborate on a codebase together.
Yes, but that's a social problem not a code problem. It doesn't mean your code was wrong, it means the process by which you integrated that code was wrong.
> Secondly, nothing is free. My code traded the ability to change requirements for reduced duplication, and it was not a good trade. For example, we later needed many special cases and behaviors for different handles on different shapes. My abstraction would have to become several times more convoluted to afford that, whereas with the original “messy” version such changes stayed easy as cake.
It's telling that there isn't a code example for these "special cases". My guess is that allowing them as configuration points in the de-duplicated calls would not have been that complicated.
I have heard this "problem" brought up before, and in my experience, it's not nearly the problem people claim it is. Yes, there is a balance to be struck: you don't want to remove duplication before you understand what's really duplicated, hence the rule of three (and I'm even fine going well beyond three). But I've yet to work on a codebase that wasn't well on the "too much duplication" side of that.
The REAL problem I see with the refactor is that the new data structures don't correspond to business objects, the way the user thinks about them.