7 ms·
I feel some people here are misunderstanding the blog post. Sandi Metz IMHO doesn't claim that the problem occurs at step 2 or 3. She doesn't claim that it's w
by Tainnor 6y ago
I feel some people here are misunderstanding the blog post.
Sandi Metz IMHO doesn't claim that the problem occurs at step 2 or 3. She doesn't claim that it's wrong to introduce abstraction when there is duplication.
What she is saying instead is that the problem occurs from step 6 onwards: when you find yourself wanting to reuse an abstraction that, regardless of whether it made sense in the first place or not, has outlived its usefulness.
I think this is in agreement with other points that she often makes, about being bold, but methodical about refactorings.
The whole discussion about "you should never abstract away code before you see the third duplication" has little to do with the article, and I'm also really not sure it's good advice.
- kwhitefoot 6y ago> Sandi Metz IMHO doesn't claim that the problem occurs at step 2 or 3. But the headline does. I had to read quite a long way down the page to discover that all she is advocating is what i have always done: deduplicate when practical, undo the duplication when new requirements make it incorrect and push the unique parts into the callers.
- DougBTX 6y ago> But the headline does. That’s not really fair, it repeatedly says “wrong abstraction”, in the title and in the article. At steps 2 and 3 it is still the right abstraction, duplication only becomes better when it is the wrong abstraction.
- tarsinge 6y agoThat’s not what I get from the article. The problem does indeed occurs at step 2 and 3: leave duplication alone and don’t introduce abstraction if you are not sure about future requirements.
- Chris_Newton 6y agoTaken to its logical conclusion, doesn’t that argument mean we would almost never introduce any abstractions at all? That doesn’t seem very practical compared to the alternative of introducing abstractions if they are useful at the time but remaining willing to change or remove them again later if the situation changes.
- tarsinge 6y agoYes my comment was poorly worded it misses something like "unless you have a strong case for it". Also there was a title change on HN, previously it was "Duplication is far cheaper than the wrong abstraction", she also says "prefer duplication over the wrong abstraction" at the beginning, so with that emphasis I might have misinterpreted the rest of the point.
- pkulak 6y agoNot to take this on a huge tangent, but I really _do_ think it's good advice. Unrolling complicated abstractions is a lot of work. Keeping two pieces of nearly identical code in sync is work too, but I've never found it all that onerous. But there's obviously a continuum; on one side it's obvious that it's a shared concept, and on the other, code just happens to be similar almost by chance, and not for much longer. But lately duplication has been turned into a code smell to be linted out, causing a lot of people to get rid of all of it, at all cost.
- lilyball 6y agoIf you only have 2 instances but are spending effort keeping them both in sync for changes then that might be a good time to abstract anyway. The fact that you’re keeping them in sync means they aren’t just coincidentally the same. But this is very situational.
- guenthert 6y agoThe cost of maintaining two (or three ...) copies might well be less than creating an abstraction. The danger rather lies in situations where not everyone potentially modifying that code (now or later) is aware that there are N copies (and where they are) which need to be maintained.
- dynamite-ready 6y agoThat's a big part of it for me. The abstraction would end up being the best way to document the duplication, imo. Far better than the likes of /* See also... */. It depends of course, but I personally feel the work of 'simplifying an abstraction', is easier than the problem of 'tracking down anything that might need to be edited'.
- watwut 6y agoThen such person needs better IDE, because finding callers is one shortcut away.
- unscaled 6y ago
- qes 6y ago> you should never abstract away code before you see the third duplication" has little to do with the article, and I'm also really not sure it's good advice Absolutes like that are rarely good advice.
- Tainnor 6y agoSure, but some comments on here are literally saying that. Not as a rule of thumb (although such comments can be found here too, which is ok), but as a "as an engineer I always enforce this rule" thing.
- lucbocahut 6y agoAbstractions have other purpose than deduplication. They make it easier to reason about your code as well. It might be the smartest thing sometimes to abstract away the first occurrence in a method imho.
- foobiekr 6y agoI think it's fair to say that abstractions should have to prove themselves as a necessity and that we make things abstract way too early. Most really good abstractions in an app fall out of well-written code to solve a specific problem. In day to day life as an engineer, I find that we have very few _enduring_ abstractions - there are very deep ones, like the concept of streams, things like filesystems and related ideas, the concept of a virtual machine in the process sense, and so on - and a lot of faddish abstractions that have a pretty wide blast radius when they start to go wrong. A lot of the good ones (networking has a _lot_ of these, such as the abstractions above and below the model of an interface in professionally-written network device code) are focused on layering.
- Tainnor 6y agoI disagree, and this is one of the things where it's really hard to get to a shared understanding because I don't know what kind of problems you've worked on and what kind of code you've seen and vice versa. But in my daily work, I routinely see abstractions just come up very naturally all the time. Sometimes they turn out to be slightly wrong, but often also not. I need to perform some calculation (e.g. for billing)? That can be abstracted away. I need to parse some unstructured user request into something structured? That's an abstraction. And so on. A lot of these things are clear to me even before I start writing code. I also tend to use (at least some amount of) DDD, to write small, composable functions with few side effects and to be as declarative as possible. All of this might help with coming up with lasting abstractions. But I'm not denying that I'm running into lots of situations in my daily work where it turns out some abstraction was wrong. Just that I find that many more of them actually turn out to be correct or at least correct for the most part (it might be that something needs to receive an additional parameter or to returns some slightly different structure to account for error conditions or so, but it's still basically the same abstraction).
- barrkel 6y agoI sometimes review PRs where people are encouraged (by other reviewers) to create new methods because there's a single line of code duplicated between two other methods. I don't think that rate of abstraction construction - and every method is another abstraction - is helpful for the health of the code, or its readability.
- BoiledCabbage 6y ago> What she is saying instead is that the problem occurs from step 6 onwards: when you find yourself wanting to reuse an abstraction that, regardless of whether it made sense in the first place or not, has outlived its usefulness. You're 100% correct in this. And what's even more amazing to me is that even after you explicitly calling this out, the majority of people replying to you (and presumably have read the article) still think the problem is between 2 & 3. The argument she is making is not "don't make abstractions until you're 100% certain they are correct". She is essentially saying make abstractions where appropriate. Some of these abstractions will be wrong. When you start seeing yourself making certain behaviors it's probably because it's the wrong abstraction, so back it out and refactor. Ultimately that abstraction seemed right based on the info known at the time it was created, now that you know more don't try to cling to it because it was already made. Be ok with backing it out and refactoring.
- qznc 6y agoIf you see an abstraction does not fit, you have the choice to consider it incomplete or unsuitable. If incomplete, you can fix it (assuming write access). If unsuitable, you should "back it out" as you say. In my opinion this distinction is applicable and thus useful in contrast to whining about leaky abstractions: http://beza1e1.tuxen.de/leaky_abstractions.html http://beza1e1.tuxen.de/leaky_abstractions.html
- Firadeoclus 6y agoIt seems to me that a straightforward fixing of an incomplete abstraction is exactly what Sandi Metz warns against (i.e. steps 5+6). The abstraction is "almost perfect", so it should not fall in the "unsuitable" category. It just so happens that complecting several slightly different uses in one abstraction comes at a significant cost. Backing out (inlining the abstraction, eliminating unused code) is a simple recipe to let you see the true amount of overlap, which may or may not itself be a suitable candidate for a smaller abstraction.
- qznc 6y agoI rather see Sandis post as a criterium when an abstraction should be considered unsuitable: When you use only a small fraction of it because of conditionals.
- barrkel 6y agoI think you generally shouldn't create an abstraction until you have at least three uses for it. That's very generally. You might want to create abstractions before then, but be prepared that they will be wrong, and don't invest in e.g. lots of unit tests, because when you break the abstraction you'll throw away that work. Some unit tests yes, but more in semi-integration tests that verify the stack sandwiching the under-proven abstraction.
- majormajor 6y agoI'm not sure it matters about applying "prefer duplication over the wrong abstraction" at step 3 or step 6 nearly as much as applying that advice at some point. I often consider "is this abstraction going to be prone to misuse" (regardless of if it's the second, third, fourth... copy) and try to head it off with either strict typing or comments or internal visibility - to try to do step 3 without opening up as big of a door to step 6, but the important thing is less when to do stuff like this but just to try to avoid things reaching step 7.
- dpc_pw 6y agoWe humans just can't help ourselves, but to invent mental shortcuts. Making a judgment "is this really a good abstraction or am I just mindlessly deduplicating code" is context-dependent, nuanced and requires some mental effort - much more work than "do I have it repeating 2 or 3 times already" which is mindless and mechanical.
- dave_sid 6y agoYou are correct that the problem lies at point 6. However a problem only exists if programmer B decides that it is acceptable to keep adding conditional logic to the method. Wrong. This is just a case of programmer B not knowing how to refactor properly. There really is nothing else to it. Yes remove duplication. Then if later on, that duplication then requires conditional logic then refactor to replace the conditional logic with polymorphism. Both these steps are clearly defined on Martin Fowler’s Refactoring book and just need to be applied when the time comes. I think the article has the premise that once you remove duplication you have committed to using that abstraction and need to shoehorn all future changes into that function. That’s a ridiculous notion. You can simply refactor it again if the abstraction no longer makes sense.
- Aeolun 6y ago> I think the article has the premise that once you remove duplication you have committed to using that abstraction and need to shoehorn all future changes into that function. That’s a ridiculous notion. You can simply refactor it again if the abstraction no longer makes sense. I don’t understand how you read this. The premise is exactly the opposite...
- oweiler 6y agoSandi is a he.
- Tainnor 6y agoI'm pretty sure that Sandi has identified as female in the past, but in case this has changed or I'm simply wrong, would you mind pointing me to a reference?
- unscaled 6y agoI also (want to) understand the article this way, but I think the article is a bit vague about this point. It focuses on Programmer X, who is coming after many iterations of trying to retrofit features into a very wrong abstraction. The article agrees that the abstraction was useful when Programmer A wrote it, but it remains vague about whether Programmer A's judgement was correct at step 2 or 3! I'm not quite sure what Sandi Metz is thinking right now, but the original 2014 talk[1] linked in the article is pretty clear about _keeping the duplication_, and _waiting for the right abstraction_. This statement sounds more in line with the Rule of Three, or the Go proverb 'a little copying is better than a little dependency', which is what people are arguing here. Personally, I believe that any non-trivial duplication should be eliminated at the very first chance, to avoid unintended business logic divergence. I find the later case more risky than having to deal with the wrong abstraction, since - as Sandi eloquently explains here - wrong abstractions can (and should!) be refactored. [1] https://youtu.be/8bZh5LMaSmE?t=893 https://youtu.be/8bZh5LMaSmE?t=893
- addicted 6y ago> but it remains vague about whether Programmer A's judgement was correct at step 2 or 3! Which emphasizes the OP’s point that the article isn’t about what happens at steps 2 or 3 at all. So it’s not concerned with whether the judgment there was right or wrong.