4 ms·
Reducing cognitive load by breaking things into digestible pieces can be hard, and at times it's hard to say if option A is better than option B, but it is not
by davvolun 7y ago
Reducing cognitive load by breaking things into digestible pieces can be hard, and at times it's hard to say if option A is better than option B, but it is not preference.
If `extractId` is an eight-step process that's only used once, but I step over it and check locals to find that the variable was correct before the call, and wrong after, then I just found my problem. That would be faster than stepping over each step to find the problem -- basically the debugging version of divide and conquer (for finding the solution, optimal would be binary search, but for readability and maintainability, that should not be the goal).
Another example is if I have a boolean condition that incorporates extremely complex logic but is only used once, if I assign it to a variable called `nameWasPreviouslySet`, that is easier to read and understand, and if I'm debugging and expect `nameWasPreviouslySet == true` at this point, and it's not, then I've figured out the bug is in setting `nameWasPreviouslySet`. So DRY shouldn't be the only reason to refactor -- refactor for readability also.
- horsawlarway 7y agoOk, 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.
- horsawlarway 7y agoOh, and I'll add in a separate comment because it's sort of unrelated. Code structure should have very little to do with how you narrow down failing code. You can do a binary search by just shoving some log statements into the application and seeing what prints out (or hell, use the debugger for your toolset). That doesn't change if I have 100 lines in a single method, or 500 lines divided out into 100 different "tiny" methods. I think it's actually harder when the methods are all split, because again - more paging back and forth in the editor to add the required debugging info.
- dragonwriter 7y agoYou are talking about what is (or is equivalent to) printf-style debugging; if you use an actual debugger, you can step over function calls, you’ve got to do more work manually setting breakpoints to do the equivalent with undifferentiated streams of code in a long function.
- horsawlarway 7y agoNo, what I'm talking about is how to narrow a problem down. The medium you use to accomplish that is flexible. I'm a little confused as you why you think clicking step-over 20 times to get out of the std::lib is better than just moving the mouse down 20 lines and adding another break. So again, this feels like we're back at "this is a matter of preference".
- hollerith 7y agoSo, you add a command to your debugger that lets you step over a "block" (a sequence of statements surrounded by braces -- the analog to Lisp's PROGN and Scheme's BEGIN) and before you debug the undifferentiated stream of many statements, you factor it into a handful of statements -- without introducing any new function names -- some of which are blocks.