> 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:
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.
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.
> 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.
In Visual studio it doesn't really make a difference from a debugging perspective. However in other debugging tools perhaps it does, and in that case I would tend to agree. But then this is a minor point, something that can be covered with a coding standard and linting the source.
Actually, yes. That's exactly what I'm advocating. Whether they are separate assignments or just chains broken up into multiple lines, the end is the same as far as what I'm talking about.
> 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.
The thing is, if you want to split the line into multiple lines you have to introduce local variables all over. I actually find this harder and more cumbersome to read because if you are reading the rest of the function, you want to know if these are used anywhere (assuming imperative language). If you have everything on a single line, then you know that the intermediate values are not used anywhere else.
I agree that the line number thing is helpful though. However, if you realize you have a bug you'll likely have to reproduce it anyway, and at that time you can introduce local variables just to see where you went wrong.
Perhaps in the end this is just subjective? This style is very common in functional programming (which I prefer) so maybe that's why I like it and hate having more local variables to keep track of.
> 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.
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.
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.
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.
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?
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.