3 ms·

> Then one of two things are likely to happen You forgot the third most common case - 90% of those repeated methods will never have a bug and now you've got a

> Then one of two things are likely to happen You forgot the third most common case - 90% of those repeated methods will never have a bug and now you've got a code base that will start smelling bad and no one is going to want to work on it. I agree with some of the things here, but allowing multiple places of repeated code unless there isn't a bug sounds like a terrible idea. Lot of these small methods will never have a bug and they'll continue to rot the codebase.

lostcolony 6y ago
Can you get the trade off right 100% of the time? Because I can tell you, every time I've worked on a codebase that repeated itself, it has been a freakin' -delight-, compared to the times where DRY was taken as a commandment from on high. The former when something broke, I could just...fix it. And it would be fixed. Would other, similar situations, still be broken? Sure! And when those would be raised up we'd fix them too, and compare them with the other changes, and possible refactor. Fixing one bug = one bug less. The latter? Oh God. Something is broken? We'd fix it. Aaaand, now there'd be two bugs. Fixing one bug = more bugs. Perfectly balanced code, yes, fixing one bug = multiple bugs fixed. That's the goal. But you won't get it right if you do it pre-emptively. Which of those other two options would you prefer?
- deleted 6y ago
  [deleted]
- throw1234651234 6y ago
  I don't even know how to comment to something like this. I guess with an example - I spent some weekend time fixing 30 file paths in a Cloud function, because the junior developer couldn't abstract the root to a single method. After you do something like that, tell me how much you love fixing bugs everywhere.
  - throwaway201103 6y ago
    Fixing the 30 file paths was not just a quick global search/replace?
    - wolco2 6y ago
      Reminds me of the time I wrote 100 similiar sites separately with slight changes. A quick global search/replace will introduce so many unnecessary replaces unless you are careful and miss so many unless you take linebreaks into account. A global search/replace is a hotfix hammer that should only get pulled out rarely and carefully by manually reviewing all changes.
    - throw1234651234 6y ago
      Bad example, it was more complicated than that. Azure functions have a different file system than App Services, and the method tried to determine where it was hosted 30 times, or something like that. Genuinely don't remember.
  - lostcolony 6y ago
    Maybe you misunderstood; I am not saying avoid DRY. I am responding to a thread wherein the parent said, essentially, "confirm that it's really repeating". My whole point was that we reach for DRY too early, when we don't know if it's actually repeating ourselves. The logic -right now- looks the same. Will it be in the future? Most of the time, we don't. Even when we think it will, we're often wrong. Your example sounds like there's literally a string literal that was not abstracted out, but hardcoded 30 places. That isn't logic. It's a literal. It has one meaning, and you can probably ascertain whether or not the meaning is the same across everything. And, as mentioned, it seems weird you couldn't grep for it. Either way though, as a counter example - I and a coworker spent three months playing whack a mole extending and supporting a rather small desktop application written by one, extremely senior developer, who had assumed 7-8 different things were basically the same, and so had written it to share a lot of the same code. They were not the same, and so a fix for one thing invariably broke two others. I was thinking of it in particular with my original post; we fixed one thing, and two more things would be broken. I got permission to rewrite the application over the course of a couple of days from the team lead, and proceeded to basically do a lot of copy paste to separate out each thing into its own control flow, where a fix to one would not affect the others. Our MTTF plunged, and within a month it was basically stable.
    - throw1234651234 6y ago
      Sorry about misunderstanding. I think that's where SRP and naming comes in like mentioned elsewhere in the thread. If methods really do one simple thing and are properly named, it's easy to tell if they should be re-used or not. A lot of the time, side-effects are not mentioned in method names or params (or the params are optional, that's where the issue comes in). E.g. SaveCandidate(Candidate candidate); In reality, SaveCandidate does two things, saves the candidate and updates their "ProfileCompletionStatus". If someone sees SaveCandidate and not SaveCandidateAndUpdateCompletionStatus or SaveCandidate(candidate, SaveSideEffects.DO_UPDATE_COMPLETION_STATUS) they will re-use SaveCandidate in places here completion status shouldn't be updated (contrived example - data migrations). The problem, of course, is that these overly-clear, overly-detailed methods are a pain to read. The common advice is "it's separate", but then we are repeating SaveCandidate(candidate); UpdateProfileCompletionStatus(candidate); everywhere, despite them being generally one process. I guess this is all pretty pedantic. When you spend a ton of time on this, you are missing out on getting stories done, but if you don't, the code eventually becomes umanageable.
  - sagichmal 6y ago
    Refactoring repeated code is toil, but cognitively trivial. Refactoring a bad or leaky abstraction can be fiendishly difficult. I'll take the toil any day.
- potta_coffee 6y ago
  I feel like I'm taking crazy pills because you describe exactly what I have to deal with on a daily basis, but I'm the only on my team that feels this way.
dhruvkar 6y ago
Actually asking, as I've never worked on a large existing codebase: If there is no bug, why would it continue to rot the codebase?
- systemvoltage 6y ago
  https://en.wikipedia.org/wiki/Code_smell https://en.wikipedia.org/wiki/Code_smell
  - oxfeed65261 6y ago
    Will you be here all week?
    - systemvoltage 6y ago
      Not sure if I follow?
      - oxfeed65261 6y ago
        I thought that your response was hilarious, and deserved a "Thank you, I'll be here all week." :)
deleted 6y ago
[deleted]
hombre_fatal 6y ago
These kinds of abstract code discussions almost become immediately absurd because, for it to work, we have to be imagining the same hypothetical codebase. Yet we never bust out concrete code. It's funny. But the outcome that you're lambasting so rudely (really? "terrible idea" when we aren't even looking at code?) is still often the best outcome. Some rotting, bugless, duplicated code is some of the easiest code to work with. It's the code you wish you had when you're debugging the complicated failing abstraction that GP wanted to avoid. The most damning thing you yourself could say about it is that it was taking up space. In fact, you seem to be making the exact reverse argument of GP: that some duplicated, unabstracted, bugless code poses such a risk ("rot") that it's a "terrible idea" to not immediately merge it into one frankenabstraction. When this happens in an argument, usually you both are imagining an absurd extreme that's opposite of the other's chosen extreme. And you actually are in agreement, as you'd both go "oh well yeah, if you go to that extreme, then I'd definitely agree with you."
- jonno123 6y ago
  I want to frame your first sentence. Abstract code discussions are absurd. I’ve learnt to mostly ignore programming blogs if they don’t include some concrete code.
  - oftenwrong 6y ago
    Most code maintenance issues don't really appear until a relatively large amount of code and code evolution is involved. It's hard to include that in a blog post while keeping it digestible for the average reader. In a small example, excessive duplication is not a problem, and excessive DRYness is not a problem. Therefore, it makes some sense to me to leave out code, and just hope the reader has personal experience to draw relevant examples from.

lostcolony 6y ago

Can you get the trade off right 100% of the time? Because I can tell you, every time I've worked on a codebase that repeated itself, it has been a freakin' -delight-, compared to the times where DRY was taken as a commandment from on high. The former when something broke, I could just...fix it. And it would be fixed. Would other, similar situations, still be broken? Sure! And when those would be raised up we'd fix them too, and compare them with the other changes, and possible refactor. Fixing one bug = one bug less. The latter? Oh God. Something is broken? We'd fix it. Aaaand, now there'd be two bugs. Fixing one bug = more bugs. Perfectly balanced code, yes, fixing one bug = multiple bugs fixed. That's the goal. But you won't get it right if you do it pre-emptively. Which of those other two options would you prefer?

deleted 6y ago
[deleted]
throw1234651234 6y ago
I don't even know how to comment to something like this. I guess with an example - I spent some weekend time fixing 30 file paths in a Cloud function, because the junior developer couldn't abstract the root to a single method. After you do something like that, tell me how much you love fixing bugs everywhere.
- throwaway201103 6y ago
  Fixing the 30 file paths was not just a quick global search/replace?
  - wolco2 6y ago
    Reminds me of the time I wrote 100 similiar sites separately with slight changes. A quick global search/replace will introduce so many unnecessary replaces unless you are careful and miss so many unless you take linebreaks into account. A global search/replace is a hotfix hammer that should only get pulled out rarely and carefully by manually reviewing all changes.
  - throw1234651234 6y ago
    Bad example, it was more complicated than that. Azure functions have a different file system than App Services, and the method tried to determine where it was hosted 30 times, or something like that. Genuinely don't remember.
- lostcolony 6y ago
  Maybe you misunderstood; I am not saying avoid DRY. I am responding to a thread wherein the parent said, essentially, "confirm that it's really repeating". My whole point was that we reach for DRY too early, when we don't know if it's actually repeating ourselves. The logic -right now- looks the same. Will it be in the future? Most of the time, we don't. Even when we think it will, we're often wrong. Your example sounds like there's literally a string literal that was not abstracted out, but hardcoded 30 places. That isn't logic. It's a literal. It has one meaning, and you can probably ascertain whether or not the meaning is the same across everything. And, as mentioned, it seems weird you couldn't grep for it. Either way though, as a counter example - I and a coworker spent three months playing whack a mole extending and supporting a rather small desktop application written by one, extremely senior developer, who had assumed 7-8 different things were basically the same, and so had written it to share a lot of the same code. They were not the same, and so a fix for one thing invariably broke two others. I was thinking of it in particular with my original post; we fixed one thing, and two more things would be broken. I got permission to rewrite the application over the course of a couple of days from the team lead, and proceeded to basically do a lot of copy paste to separate out each thing into its own control flow, where a fix to one would not affect the others. Our MTTF plunged, and within a month it was basically stable.
  - throw1234651234 6y ago
    Sorry about misunderstanding. I think that's where SRP and naming comes in like mentioned elsewhere in the thread. If methods really do one simple thing and are properly named, it's easy to tell if they should be re-used or not. A lot of the time, side-effects are not mentioned in method names or params (or the params are optional, that's where the issue comes in). E.g. SaveCandidate(Candidate candidate); In reality, SaveCandidate does two things, saves the candidate and updates their "ProfileCompletionStatus". If someone sees SaveCandidate and not SaveCandidateAndUpdateCompletionStatus or SaveCandidate(candidate, SaveSideEffects.DO_UPDATE_COMPLETION_STATUS) they will re-use SaveCandidate in places here completion status shouldn't be updated (contrived example - data migrations). The problem, of course, is that these overly-clear, overly-detailed methods are a pain to read. The common advice is "it's separate", but then we are repeating SaveCandidate(candidate); UpdateProfileCompletionStatus(candidate); everywhere, despite them being generally one process. I guess this is all pretty pedantic. When you spend a ton of time on this, you are missing out on getting stories done, but if you don't, the code eventually becomes umanageable.
- sagichmal 6y ago
  Refactoring repeated code is toil, but cognitively trivial. Refactoring a bad or leaky abstraction can be fiendishly difficult. I'll take the toil any day.
potta_coffee 6y ago
I feel like I'm taking crazy pills because you describe exactly what I have to deal with on a daily basis, but I'm the only on my team that feels this way.

dhruvkar 6y ago

Actually asking, as I've never worked on a large existing codebase: If there is no bug, why would it continue to rot the codebase?

systemvoltage 6y ago
https://en.wikipedia.org/wiki/Code_smell https://en.wikipedia.org/wiki/Code_smell
- oxfeed65261 6y ago
  Will you be here all week?
  - systemvoltage 6y ago
    Not sure if I follow?
    - oxfeed65261 6y ago
      I thought that your response was hilarious, and deserved a "Thank you, I'll be here all week." :)

deleted 6y ago

[deleted]

hombre_fatal 6y ago

These kinds of abstract code discussions almost become immediately absurd because, for it to work, we have to be imagining the same hypothetical codebase. Yet we never bust out concrete code. It's funny. But the outcome that you're lambasting so rudely (really? "terrible idea" when we aren't even looking at code?) is still often the best outcome. Some rotting, bugless, duplicated code is some of the easiest code to work with. It's the code you wish you had when you're debugging the complicated failing abstraction that GP wanted to avoid. The most damning thing you yourself could say about it is that it was taking up space. In fact, you seem to be making the exact reverse argument of GP: that some duplicated, unabstracted, bugless code poses such a risk ("rot") that it's a "terrible idea" to not immediately merge it into one frankenabstraction. When this happens in an argument, usually you both are imagining an absurd extreme that's opposite of the other's chosen extreme. And you actually are in agreement, as you'd both go "oh well yeah, if you go to that extreme, then I'd definitely agree with you."

jonno123 6y ago
I want to frame your first sentence. Abstract code discussions are absurd. I’ve learnt to mostly ignore programming blogs if they don’t include some concrete code.
- oftenwrong 6y ago
  Most code maintenance issues don't really appear until a relatively large amount of code and code evolution is involved. It's hard to include that in a blog post while keeping it digestible for the average reader. In a small example, excessive duplication is not a problem, and excessive DRYness is not a problem. Therefore, it makes some sense to me to leave out code, and just hope the reader has personal experience to draw relevant examples from.