4 ms·

This is extremely validating to read. How many times have I battled with DRYists over which solution is "better." I've happened upon the pattern of code growth

This is extremely validating to read. How many times have I battled with DRYists over which solution is "better." I've happened upon the pattern of code growth described here after years of encountering and resolving pain points in code, often as the maintainer. DRYness for DRYness sake might feel satisfying when writing the code, but when changes need to be made, it often scatters constraints and requirements throughout the codebase, making any change an unestimatable mess of sneaky traps. Try writing your boilerplate initialization code straight some time... no metaprogramming, no helper functions, just get your configuration, create your objects and wire them together. It is oddly liberating to have your bootstrap code flat and unmagical.

mathgeek 11y ago
> DRYness for DRYness sake might feel satisfying when writing the code, but when changes need to be made, it often scatters constraints and requirements throughout the codebase, making any change an unestimatable mess of sneaky traps. This also applies to "one-liners" when writing code. For example in ruby: Model.where(...).foo { ... }.flatten.bar { ... }.join(...) Looks nice and all, but it can be confusing when you need to fix a bug that you aren't sure of the source (foo or bar? maybe we shouldn't flatten yet? maybe we should join sooner?). It's much better, IMHO, to write more lines of code here so that you can easily test each step manually when a bug occurs.
- GavinMcG 11y ago
  I'm not addressing your overall point, but one thing I learned about recently that could help in debugging those Ruby one-liners is the `tap` method. You can stick it in the chain and set a breakpoint. http://ruby-doc.org/core-2.2.3/Object.html#method-i-tap http://ruby-doc.org/core-2.2.3/Object.html#method-i-tap
  - tvon 11y ago
    Further off-topic, but I almost exclusively see tap used as some kind of clever way around a return line, e.g. instead of: def build_hash_or_whatever hash = Hash.new hash[:foo] = some_generating_method hash[:bar] = "some value" if whatever? # More hash construction nonsense hash end With tap it is: def build_hash_or_whatever Hash.new.tap do |hash| hash[:foo] = some_generating_method hash[:bar] = "some value" if whatever? # More hash construction nonsense end end I think the tap-form is significantly worse and I don't understand the aversion to a single line that clearly shows what you are returning.
  - plaguuuuuu 11y ago
    Changing code to enable debugging is barbaric. Tooling should be able do that. Or maybe I'm spoiled, lol.
- joshka 11y ago
  Couldn't agree more. Writing a single line of code that can fail multiple ways (especially the same way on multiple parts) is one of my constant source of annoyances. Not from a DRY perspective, but from your code has a bug, but I can't give you enough detail to immediately fix it because it could be one of these X reasons. Especially on production systems, these become the sorts of issues where the developer makes some temporary fixes to the code, then has wait to observe the bug again to work out which line is broken.
- chairleader 11y ago
  Funny you mention it, I'm also a bit anti-chaining for the same reasons. Do we need a cute name to describe the practice of optimizing code for maintainability? "Craftsmanship" speaks to the building process, what should we call it when we make code that is understandable and straightforward to change?
- bbcbasic 11y ago
  Your point seems to be more about using chains of set operators rather than the fact that it is on one line. I am a C# guy and I find Linq which is the equivalent to this insanely useful and I often write code as above. The tax on this is the rest of the team need to be up to speed in reading such code. If they are then what you gain is use of a bunch of predictable operators rather than loops with counters, state, etc... that could have subtle bugs. Using code like this is no different to writing SQL that does joins and aggregates in a stored procedure and calling it from your code, something I also thing isn't a bad thing. Debugging can be done by splitting it into lines, or using a decent enough debugger. Or making it pure and writing unit tests or property-based tests to cover it.
  - mathgeek 11y ago
    > Your point seems to be more about using chains of set operators rather than the fact that it is on one line. It isn't, though. My point is that sticking chains on one line makes it difficult to read as well as debug the various bits and pieces that all rest on a single line. Consider the task of debugging any arbitrary error where you have little more than a line number to go by. You now need to refactor the line just to find out which method invocation is causing the error. There are also readability issues with lines like these.
    - lmm 11y ago
      > Consider the task of debugging any arbitrary error where you have little more than a line number to go by. You now need to refactor the line just to find out which method invocation is causing the error. That's a tooling issue. The right thing is to enhance the stack trace to give more than a line number, and the debugger to allow a more specific breakpoint. > There are also readability issues with lines like these. Maybe, but the biggest readability issue by far is when a class or function spills beyond a single screen. Avoiding that is worth a lot of cramped lines.
      - mathgeek 11y ago
        At this point this is boiling down a subjective argument on readability and tradeoffs in tooling, so no point in continuing. Have a great day!
lostcolony 11y ago
One of the earliest experiences I had as a professional software developer was taking over an app that had originally been developed by someone with 30 years of experience. He had looked at the problem domain, said "these things are all similar", and so had constructed his code so that every one of those things shared a huge amount of their code paths. It worked for the two, three things he actually coded, but as soon as I and another dev started to add more (around 12 total I think), it got hackier and hackier. Because the conditionals needs to distinguish the two or three he had originally handled were easy to ignore, but the conditionals needed to distinguish the 12 were littered all over the place, with no real predictivity to them. We attempted to isolate it as best we could, pulling out into new methods and such, but it was bug whack-a-mole; we reliably were finding a new bug within a week of each fix we made, nevermind any we introduced with new features, because trying to isolate and fix one of the things invariably broke one of the others. So, finally, I took two days and rewrote the shared logic of the application (only around ~10k lines). I borrowed liberally from the existing code, but basically isolated each thing entirely from the others. They still had the same structure, same interface, but rather than all inheriting the same implementation, they all had their own copy of the implementation, which in turn meant each one was far simpler, and could be tweaked in isolation. That took about two weeks of use to flush out the bulk of the bugs. And after that, our mean time to failure kept improving; we went with maybe one bug the third week, then one bug a month for the next two months, then it essentially stabilized; it was only in introducing new features that we'd occasionally find/introduce new bugs. That whole experience taught me not to worry about DRY until you actually have done the work. DRY is to ease maintenance and future development; it should not be a consideration for that initial implementation of a given feature. Because things that look similar enough to share functionality between often aren't once you become more familiar with the domain. If they turn out to be related enough to share functionality, it was trivial to not repeat yourself (copy/paste), and it's trivial to refactor. If they turn out not to share functionality, the effort to try and make them share code was higher than if you'd done them separately, and the amount of effort to pull them apart is huge.
- JamesBarney 11y ago
  It seems to me what you're describing was a deep and wide inheritance tree. I hate those too. In my experience they have always been very brittle, and lead to confusing code. I'm a huge fan of the recent movement to prefer composition over inheritance.

mathgeek 11y ago

> DRYness for DRYness sake might feel satisfying when writing the code, but when changes need to be made, it often scatters constraints and requirements throughout the codebase, making any change an unestimatable mess of sneaky traps. This also applies to "one-liners" when writing code. For example in ruby: Model.where(...).foo { ... }.flatten.bar { ... }.join(...) Looks nice and all, but it can be confusing when you need to fix a bug that you aren't sure of the source (foo or bar? maybe we shouldn't flatten yet? maybe we should join sooner?). It's much better, IMHO, to write more lines of code here so that you can easily test each step manually when a bug occurs.

GavinMcG 11y ago
I'm not addressing your overall point, but one thing I learned about recently that could help in debugging those Ruby one-liners is the `tap` method. You can stick it in the chain and set a breakpoint. http://ruby-doc.org/core-2.2.3/Object.html#method-i-tap http://ruby-doc.org/core-2.2.3/Object.html#method-i-tap
- tvon 11y ago
  Further off-topic, but I almost exclusively see tap used as some kind of clever way around a return line, e.g. instead of: def build_hash_or_whatever hash = Hash.new hash[:foo] = some_generating_method hash[:bar] = "some value" if whatever? # More hash construction nonsense hash end With tap it is: def build_hash_or_whatever Hash.new.tap do |hash| hash[:foo] = some_generating_method hash[:bar] = "some value" if whatever? # More hash construction nonsense end end I think the tap-form is significantly worse and I don't understand the aversion to a single line that clearly shows what you are returning.
- plaguuuuuu 11y ago
  Changing code to enable debugging is barbaric. Tooling should be able do that. Or maybe I'm spoiled, lol.
joshka 11y ago
Couldn't agree more. Writing a single line of code that can fail multiple ways (especially the same way on multiple parts) is one of my constant source of annoyances. Not from a DRY perspective, but from your code has a bug, but I can't give you enough detail to immediately fix it because it could be one of these X reasons. Especially on production systems, these become the sorts of issues where the developer makes some temporary fixes to the code, then has wait to observe the bug again to work out which line is broken.
chairleader 11y ago
Funny you mention it, I'm also a bit anti-chaining for the same reasons. Do we need a cute name to describe the practice of optimizing code for maintainability? "Craftsmanship" speaks to the building process, what should we call it when we make code that is understandable and straightforward to change?
bbcbasic 11y ago
Your point seems to be more about using chains of set operators rather than the fact that it is on one line. I am a C# guy and I find Linq which is the equivalent to this insanely useful and I often write code as above. The tax on this is the rest of the team need to be up to speed in reading such code. If they are then what you gain is use of a bunch of predictable operators rather than loops with counters, state, etc... that could have subtle bugs. Using code like this is no different to writing SQL that does joins and aggregates in a stored procedure and calling it from your code, something I also thing isn't a bad thing. Debugging can be done by splitting it into lines, or using a decent enough debugger. Or making it pure and writing unit tests or property-based tests to cover it.
- mathgeek 11y ago
  > Your point seems to be more about using chains of set operators rather than the fact that it is on one line. It isn't, though. My point is that sticking chains on one line makes it difficult to read as well as debug the various bits and pieces that all rest on a single line. Consider the task of debugging any arbitrary error where you have little more than a line number to go by. You now need to refactor the line just to find out which method invocation is causing the error. There are also readability issues with lines like these.
  - lmm 11y ago
    > Consider the task of debugging any arbitrary error where you have little more than a line number to go by. You now need to refactor the line just to find out which method invocation is causing the error. That's a tooling issue. The right thing is to enhance the stack trace to give more than a line number, and the debugger to allow a more specific breakpoint. > There are also readability issues with lines like these. Maybe, but the biggest readability issue by far is when a class or function spills beyond a single screen. Avoiding that is worth a lot of cramped lines.
    - mathgeek 11y ago
      At this point this is boiling down a subjective argument on readability and tradeoffs in tooling, so no point in continuing. Have a great day!

lostcolony 11y ago

One of the earliest experiences I had as a professional software developer was taking over an app that had originally been developed by someone with 30 years of experience. He had looked at the problem domain, said "these things are all similar", and so had constructed his code so that every one of those things shared a huge amount of their code paths. It worked for the two, three things he actually coded, but as soon as I and another dev started to add more (around 12 total I think), it got hackier and hackier. Because the conditionals needs to distinguish the two or three he had originally handled were easy to ignore, but the conditionals needed to distinguish the 12 were littered all over the place, with no real predictivity to them. We attempted to isolate it as best we could, pulling out into new methods and such, but it was bug whack-a-mole; we reliably were finding a new bug within a week of each fix we made, nevermind any we introduced with new features, because trying to isolate and fix one of the things invariably broke one of the others. So, finally, I took two days and rewrote the shared logic of the application (only around ~10k lines). I borrowed liberally from the existing code, but basically isolated each thing entirely from the others. They still had the same structure, same interface, but rather than all inheriting the same implementation, they all had their own copy of the implementation, which in turn meant each one was far simpler, and could be tweaked in isolation. That took about two weeks of use to flush out the bulk of the bugs. And after that, our mean time to failure kept improving; we went with maybe one bug the third week, then one bug a month for the next two months, then it essentially stabilized; it was only in introducing new features that we'd occasionally find/introduce new bugs. That whole experience taught me not to worry about DRY until you actually have done the work. DRY is to ease maintenance and future development; it should not be a consideration for that initial implementation of a given feature. Because things that look similar enough to share functionality between often aren't once you become more familiar with the domain. If they turn out to be related enough to share functionality, it was trivial to not repeat yourself (copy/paste), and it's trivial to refactor. If they turn out not to share functionality, the effort to try and make them share code was higher than if you'd done them separately, and the amount of effort to pull them apart is huge.

JamesBarney 11y ago
It seems to me what you're describing was a deep and wide inheritance tree. I hate those too. In my experience they have always been very brittle, and lead to confusing code. I'm a huge fan of the recent movement to prefer composition over inheritance.