3 ms·
Ok, so lets take your example at face value. You have an extremely complex step of code that you only call once. Already, you have invariants that are easy to
by horsawlarway 7y ago
Ok, so lets take your example at face value. You have an extremely complex step of code that you only call once.
Already, you have invariants that are easy to break. You're assuming it's only called once. That's relatively safe if you're inline, that's bogus as soon you've broken it out into a named function.
Some other dev WILL come along and re-use your helpfully broken out code somewhere, and that invariant no longer holds.
Even assuming no one else has re-used it, what ensures that the problem is actually in that method? Particularly if that method itself calls out to many small named methods.
So you have some complex code that does something, but the "things" it does are call out to other small chunks of code.
so now you have
DoComplexThings => {
DoSubThingA
DoSubThingB
DoSubThingC
DoSubThingD
DoSubThingE
return
}
So you're back to binary search - The problem is in there somewhere, but "in there" is actually calls out to many other functions again. Which one is broken?
And wait! DoThingD got changed at some point and will now fail if you run it before DoThingC, but there's nothing in the names that tells you that.
So you have hidden deps between chunks of code that are all trying to accomplish a single goal.
If you're debugging that, you almost certainly have to go through all the code in that method again, except you have to page back and forth in your editor to get the meaningful pieces into view. All so some OTHER dev could give it name that was meaningful to them, but likely not that helpful when you're debugging because the mental model they have of the system resulted in the bug in the first place.
---
Now, rant aside - I think we agree more than we disagree, I think good names matter, and I think there's a lot of wiggle room around when a thing should be broken out.
But if the code is trying to do a single thing, and it's not re-used, I'll take a single cohesive method over 8 tiny abstractions ANY day.
- davvolun 7y ago> if the code is trying to do a single thing What is a single thing? Login? Initialize db? One line of code, 3 lines of code, ... If you have 100 lines of code and 6 nestings, even if none of that is or, as far as you can tell, should ever be re-used, you should break that into smaller, digestible chunks of DoSubThingA, etc. It's kind of an aside, but you mentioned e.g. `DoThingD` got changed or "some other dev WILL come along" -- if some other dev comes along and doesn't bother to check where and in what context `DoThingD` is being used, and changes it in a way that breaks `DoThingC` or `DoThingE`, they aren't doing their job. Particularly, if `DoThingD` wasn't written to be re-used, and they just use re-write it and use, particularly in a way that breaks its original invariance... I would be having strong words with that developer.