7 ms·

While the example in the article isn't great for making the point I actually agree with the main idea. I understand the arguments for 'Uncle Bobifying' code bu

• quasi 7y ago
  Isnt this just 'good coding practices'? How is 'Avoid Indirection in Code' justified?
  • UglyToad 7y ago
    Yes, but 'good coding practices' is relative/subject to interpretation. I see this as a necessary counterpoint to the 'Clean Code to extremes' in a lot of other blog posts. It simply calls for some consideration before moving every little thing into another method. Granted the title is clickbaity but that's just the curse of the modern web.
• guitarbill 7y ago
  Agreed, although I think the example isn't too bad. I often see such "helper methods" - usually in a class called "utils" or "misc" - when (novice) programmers "try to plan ahead": how should I write this so it can be re-used? But KISS is remains a good principle. Predicting the future is hard. If in doubt, don't put in the extra indirection/abstraction. If/when the next person needs that functionality, they can refactor it - and the simpler/more straightforward the code, the easier it is to refactor! When the logic is already split over multiple methods, refactoring in a different direction becomes harder. Often, it's enough to verify there's only one caller, and then stuff it back into one method before refactoring. Still, more annoying than if they hadn't of overthought it.
  • usrusr 7y ago
    In my mind I contrast "keep it simple" with "make it simple". Successful "make it simple" is better than "keep it simple", which in turn is much better than failed "make it simple". Every attempt at "make it simple" is a wager. Regarding the one-caller method, beloved by followers of maximum method length ideology, scorned by enemies of excessive indirection, I think we lost a very useful tool when the "structured programming" of Pascal, which prominently featured nested functions as an organisation principle disappeared. It got steamrolled by OOP name scoping (which tends to be too big for this use case) from the top while the archaic "if it exists, it is accessible" of C firmly held the lower levels. The current trend of retrofitting functional features is technically getting us back what we lost, but the use of nested functions as a simple organisational tool is still far from idiomatic. E.g. in Java>=8, when you see a lambda assigned inside a method you expect something clever functional to follow, not a simple "I put this block of code in a box to give it a name, but it won't be meaningful outside this method".
    • JamesBarney 7y ago
      C# has local functions which I've been making a lot of use for this exact purpose.
    • blt 7y ago
      I really like this pattern, but unfortunately some people consider it confusing when the closure capture is not explicit. C++ lambdas are nice in that regard.
    • guitarbill 7y ago
      agree, like you said it's probably a teaching thing. in e.g. python, javascript, and others, nested functions are available and first-class citizens. (okay, those languages have other issues) really, there isn't a substitute for experience, but we can try. and again, the devs who do this mean well, they just don't know better yet. that's still better than apathy
  • UglyToad 7y ago
    The other thing you see is extraction of logic applied, particularly to business logic, prior to the actual logic being clear. So a developer will notice two very similar bits of code and maybe include some kind of parameter to a method or even worse, build some kind of abstraction (inheritance shudder) then as the project evolves it turns out there are actually very different business requirements for each case and you're stuck in a bit of a mess. As you say there's a cost to optimization, the wrong abstraction is far more painful than no abstraction. This quote from another blog post the other day is relevant I think: "Future thinking is future trashing When developers try to solve a problem, they sometimes try to find a way that will solve all the problems, including the ones that may appear in the future. But here is the thing: The problems from the future will never come and you'll end up either having to maintain a huge behemoth of code that will never be fully used or you'll end up rewriting the whole thing 'cause there is a shitton of unused stuff. Solve the problem you have right now. Then solve the next one. And the next one. At one point, you'll realize there is a pattern emerging from those solutions and then you'll find your "solve everything"." These are all just re-statements of KISS and YAGNI but it's definitely the most important principle.
• JamesBarney 7y ago
  The issue is fundamentally abstractions leak. And if the code is overly "Uncle Bobified" the abstractions will leak bugs. On the other hand if you under "Uncle Bobify" the code will be very difficult to read because you won't be able see the forest for the trees. This is one of the advantages of comments and local functions. You can inline a function an add a comment. With the comment providing the abstraction and the line of code providing the details. Or add a local function which is easy to read but can be easily seen next to the code that's using it. For instance you might have a function that is int getOverlap(DateTime start1, DateTime end1, DateTime start2, DateTime end2). Looks pretty reasonable. But there are a lot of unanswered questions like Is the return value in seconds, minutes or ms? What happens if there is no overlap? Do all the date time have to be in UTC? What happens if we pass in a start time before the end time? What happens if I pass in a start time equal to an endtime? If I split a block will the overlap of this block with any other block match the combined overlap of any two split blocks?
  • blub 7y ago
    All of those questions can be answered through comments. I'd vote for a separate function. Some of the answers can be encoded in the type system through the use of time units (like std::chrono), optional return values and design by contract (preconditions). Unit tests could be added to further clarify some edge cases.
  • skohan 7y ago
    I think most of those questions could be answered with a sufficiently strong type system.
  • lmm 7y ago
    > The issue is fundamentally abstractions leak. Only people who've never used a good type system think this. Your function example demonstrates this pretty nicely: use a proper interval type for the two intervals and a proper return type rather than int, and then the answers to all your questions become obvious.
    • JamesBarney 7y ago
• rhizome31 7y ago
  We should extract small functions when it helps understanding the program. Sometimes the code is clearer when we extract a one line function - non-trivial boolean expressions come to mind - and sometimes we should keep a (reasonably) long method that is better understood as it is. As always we should adapt our approach to the situation and not blindingly apply some preconceived rule. One size does not fit all. But generally what I see in code bases is a need for cleaner code. I think real-world code suffers more from being not "clean" enough than from being too "clean". That's why I'm a bit wary of this kind of articles that can be interpreted as advocating for lower standards. It's a bit similar to articles criticizing the excess of TDD or mock-based testing. They might have a point in already mature contexts but the reality of most business software is the big ball of untested mud. We're not all DHH or Matthew Rocklin and what I see around me is a need for cleaner code and more testing, not the other way around.
  • aib 7y ago
    > We should extract small functions when it helps understanding the program. This. If I may reiterate your point: `is_foolike(x)` may not be much better than `x.startswith("foo")` but, say, `is_superuser(x)` is much easier to understand than `x.username.starts_with("su_")`. And note how the mental burden is decreased in this case as well, as a first-time reader would not be wondering why we're checking usernames in the middle of a function way past login.
• jerf 7y ago
  "While it's useful to have it split up into logically discrete chunks with sensible naming these 2-4 line functions far increase the mental load of the code." I don't do this a lot, but in those cases where I'm tempted to pull out "helper functions" that I just call once, simply so I can give them a name, I often use blocks instead. That is, instead of: func DoTheThing(...) { x = doTheFirstPart(...) y = doTheSecondPart(...) return doTheThirdPart(x, y) } func doTheFirstPart(...) { ... } func doTheSecondPart(...) { ... } func doTheThirdPart(...) { ... } I'll do: func DoTheThing(...) { // Do the first part var resultType1 x { // basically the contents of doTheFirstPart here } // Do the second part var resultType2 y { // basically the contents of doTheSecondPart here } // Do the third part { // doTheThirdPart here return final result } } If you're literally going to call the function only once, and it just disrupts the flow, this has a number of advantages. In most brace-using languages, the braces will confine a scope just like a function would have, so you still have clear specification of what can flow out of the brace functions via the previous variable declarations. (They can stack up a bit if you have a lot of these sections but I've not had too much trouble with that.) If you do need to pull it out into a function later, it's a fairly mechanical process instead of a surgical process, because you're already 90% of the way there. But if you never need to do that, you retain the ability to simply read through a function and see everything it's doing. I've noticed a lot of my "payload" functions sometimes end up long, but if you look carefully, you'll see I'm still using a lot of tools for scoping and simplification to confine action-at-a-distance within the function itself. You do get a couple of weird looks and possibly some weird review comments if you use this; people are not used to seeing scopes used solely for isolation like this. I've had people ask me whether this is a syntax error or not in the review. But usually once you explain yourself it passes through. This seems to mostly end up in my testing code, where I may need a chunk to set up the environment for a particular chunk, and then may have some tests I want mostly isolated from each other, but to use the same environment, or in a recent case, I had a chunk that setup the unique environment for the test, the code under test was basically "testEnvironment.DoIt(...)", and then I had half-a-dozen blocks verifying that the DoIt call had done all the things it was supposed to do and had all the exact side effects it was supposed to have, one class of them at a time, confined in braces so it's syntactically enforced they can't have leaking values. (This was an integration-level test.) There are definitely other cases where simply naming things and breaking them into functions can be helpful, so you end up with an "orchestration" function that reads cleanly, and all the little bits also read cleanly. I do that plenty too.