It is not very useful to break up long sequences of imperative commands (procedures) because chances are the procedure chunks you end up with are completely ad-hoc and non-reusable anyway. Also, as John Carmack puts it, "you should be made constantly aware of the full horror of what you are doing".

For pure functions, it's often a bit of a smell if they're very long; usually you can decompose them into smaller functions which are actually reusable, and you end up with small units that can be tested in isolation and are very easy to understand.

Generally speaking (not a hard rule), it's a win to have more of your program written as pure functions. But it cannot always be done in a sane way, in that case, write procedures and write them long.

I really came to appreciate this while ramping up on test writing. Decisions are often trivial to decompose, reuse, and test. Actions not so much, and are sometimes defined by procedures in the literal sense.

Still, if you have facility with the English language or aren’t ashamed to use a thesaurus, meaningful method names and boundaries can help break a problem down and let someone zero in on a bug.

The approach of putting all of the logic inline into the same method is much preferable to me over the alternatives. The point at which I would turn a new method is when a block of code is executed the same way in 2 or more contexts within the same class. Otherwise, I see no reason to break out the code, unless it is making readability difficult in a fairly obvious way - e.g. an inline new List<MyParameter>() which is 100 lines long should really be under a GetMyParameters() private convenience method. Other candidates could be similar points of control (e.g. make it easier to find where to control parameter listings by having them in their own named methods), or managing wildly different levels of abstraction (mapping code vs business logic, maybe create separate mapping methods).

I've taken this a step further and started collapsing entire layers. For instance, we'd have a Repository and Service abstraction, but what ultimately happened is there was only ever 1 concrete implementation of each, so these ended up combined. Something like MyBusinessService+MyBusinessRepository were reduced to a combined MyBusinessService. This has the advantage of allowing you to see exactly how the business logic is manipulating the underlying datastore by reading a single consolidated method. The combined implementation is certainly larger overall, but taking out the context switch far outweighs this minor downside in practice.

Productivity is king. I have given up on trying to have code that looks clean simply for the sake of aesthetics.

1. I would always advocate for testing full-stack with the datastore in the loop. E.g. an actual API request from a test runner all the way down into a test instance of Postgres. Isolating your testing to specific layers is clever, but I find 99% of my errors come from forgetting to adjust SQL queries after migrations were performed, or other things that are not going to get caught in the BL layer. If you did your BL correctly using the latest C# 8.0 primitives, you get a lot of compile-time guarantees of correctness (e.g. null enforcement throughout), so the value of unit testing these layers is diminished from my perspective.

2. The value of architectural abstractions changes over time. I would certainly advocate for a temporary IRepository injected into a service implementation if it makes the initial 100 iterations of the code faster by way of using a LINQ-to-objects shim in place of Postgres. But, once this code has been running stable in production for months without concern, perhaps its time to take out these abstractions so that the people responsible for maintaining the code on a daily basis have less complexity to deal with.

By splitting code into functions you have a more well defined scope for each of them and they can be understood in isolation.

You also get the benefit of getting a quicker overview (if you care to name your functions well) in your calling function (DoAll).

Then there is the issue of maintainability. When I have to change something in a big (in-lined) function then I'm much more anxious about the whole context. My instinct is often to refactor it into smaller pieces so I can narrow down the changes and isolate them correctly and exactly from the context.

This might very well be a thing of preference?

On top of arbitrarily pushing the system in directions, you yourself note that you hide the actual context of the code when you split it up. It might make you feel better in the moment but I don't think it's the right response to feel emboldened by making decisions with less context. Premature abstraction is at the heart of a lot of bad design and complexity.

With even primitive dev tools you can get a lot of the benefits (grouping, naming) with comments and braces. More could definitely be done on this front but dev tool progress is sadly pretty slow. Going this route you can have organization while not throwing away the all-important context of what the code is actually meant to accomplish.

Personally, I really dislike pulling everything out into small methods - it literally forces you to remember a crap load of names. I find remembering names to be extremely high cognitive load.

Worse, the names WILL lie to you. Especially if you weren't the one doing the naming.

So not only are you forced to remember a bunch of labels that better fit someone else's mental model, you're still not off the hook for understanding what the code in those tiny methods is doing, and considering how it might impact the task at hand.

So now you're stuck with twice the work - Remembering which name goes with which functional piece of the task at hand, and then remembering how that name actually accomplishes the task (the actual code).

Worse, because functions are moved out of line, I find it much harder to jump between relevant bits of actual "doing things" code.

Basically - The ONLY time I want to split code off into named chunks is when the alternative is copying/pasting code somewhere. I look for code which is getting reused and break that out.

The in-lined version is faster to read, faster to understand (and by understand I mean REALLY understand, not some hand-wavey "I trust this name" understand, but as in you actually know the operations and changes to the system that the call will result in)

The downside to inline code is you have to actually read it. I find a lot of the folks who really like short methods struggle to parse the language they're working in and fall back on the name without actually understand what the chunk of code does. But I think they're also the folks who have a better memory for names.

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.

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.

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.

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.

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".

Folds get a bad rap because they aren't that widespread, outside of Emacs/Vim and amusingly, the .NET world, and because some people abuse them to write God-classes and such.

You get the niceties of a name for the folks who want it, but the code is still grouped nicely and it's easy to walk through the whole operation.

I see this much less often in languages where scoping is less flexible though.

Forcing stuff to sub functions just hiding information away from the people that reads code.

DoA(DoAllContext ctx);

This is, of course, when we're talking about programming in Objects.

For most of the last 10 I've been working in Python.

http://number-none.com/blow/blog/programming/2014/09/26/carm...

Literally any testing strategy can be forced to work for about 18 months. Anyone working with shorter lifetimes than that is inexperienced.

    DoAll() {
      // A
      a1 = ...
      a2 = ...
      a3 = ...

      withOnly a1, a3:
      // B
      b1 = f(a1)
      b2 = g(a3)
      
      withOnly b2:
      // C
      c1 = h(b2)
    }

For example:

   doThing(){
      x = 7; //x visible in whole function
      {
         y = x*2; //y only visible in this block.
      }
      // x still visible, y no longer visible
   }

Many small bits of logic are broken out into their own methods (checking if today is a public holiday, configuring the HTTP server, configuring the KV store client, etc), but the setup of the domain logic components is all in one method.

There's no way to split the setup of the components into methods without either introducing a comparable volume of boilerplate, or having to write some kind of framework to make it easier. Neither would be a win.

A caveat: this codebase uses a language which doesn't have out parameters or multiple-value returns. If it could use those, the boilerplate involved in breaking up the setup method would be considerably smaller. But still probably not worth it.

    DoAll() {
      // A
      a1 = ...
      a2 = ...
      a3 = ...

      // B
      b2 = lambda(x,y) {
        b1 = f(x)
        b2 = g(y)
      }(a1,a3);
      
      // C
      lambda(x) {
        c1 = h(x)
      }(b2);
    }

1. If you have good reasons to restrict variable access in subsections of your long procedure, then it DOES make sense to factor them out as separate functions.

2. Closures automatically close over variables from the surrounding scope (duh), so they don't protect you from accidentally using a variable in a section that you're not supposed to use it in.

Which isn't a problem we should be avoiding. Naming things in a way that other coders will understand is an important skill, and can be difficult.

The lambda doesn't help me understand what that part actually does any more than calling the function "foo_helper" or "foo_partN" or something like that would do.

I believe we're actually arguing the same thing.

Anonymous functions aren't the end of the world, but self-documenting code is better.

In the JS/JQuery world, breaking away from anonymous functions would represent such a fundamental shift in practice, I'm not going to do it on my own, but in Angular, I stay away from anonymous functions.

* Code folding is not the same thing, as it doesn’t eject scope+args out of the “caller” and cannot be used in expression.

Upd: I also would like to add that sources lack usual style formatting. //-sections usually should stand out, but one can only make comment bigger (2-3 lines with === filler), but not the text itself. It would be nice to have some rtf in code and css in IDE. Books are written like that, and that’s well-acknowledged way to structure instructions and descriptions.

If I understand what you mean, Visual Studio Code allows this. I guess Visual Studio does as well.

The implementation feels a little clumsy to me, but hey, I'm the guy who thinks Netbeans and Eclipse where the best IDEs I've used, so I guess a bunch of you guys will think the solution is perfect.

First, you have “a hundred lines of code”

Then, you add white space to separate somewhat independent parts.

Then, you add comments that describe those parts.

Then, you notice some of these comments aren’t good, and refactor the code a bit and tweak the comments to make them good comments.

Once you’re there, refactoring the parts into decent functions is just a further iteration. It may be hard work, though, as it often means introducing new types that must be given good names.

Where you stop with this is a bit a matter of taste, but also depends on project size, development team size, (expected) longevity of the code base, etc.

That's because separating them out into separate procedures enforces a simple interface: communication only happens via arguments and return values.

When a reader sees a block like

    DoAll() {
      // A
      ...
      // B
      ...
      // C
      ...
    }

In C-style languages you can use {}-delineated blocks to get most of this benefit of the short-method style in the long-method style.

In eg Python you can use nested functions to achieve your readability goals of not having to jump around while still keeping your state from interacting.

The reasoning isn't so bad. Complexity is a function of length and communication. Complexity is the enemy. When you optimise for speed, the other side of the equation is always complexity. Or: if you can decrease complexity and increase speed it's always a win.

Spitting code up in your example up makes it obvious the steps are utterly independent. In fact if there are go global variables those steps can be done in any order - or indeed in parallel. Since splitting it up makes that fact self evident whereas to learn it before you had to read the code, it has reduced complexity.

But of course you almost never split it up as you have done, because there is always a tradeoff between function length and communication. Reduce one you almost always increase the other.

Worse, communication is a multi-faceted beast, so measuring its contribution to complexity is hard. Communicating by passing parameters to pure functions increases complex less that communicating by global variables for instance. And can't check communication complexity using code length - if you pass a single value "a" that is in fact just a copy of the local variables, you've achieved nothing! Using length as a measure for complexity is equally fraught. If DoA() is a single like of code, I doubt anybody would say you've improved things much. But if DoA() was, unlike your illustration, lines intermixed it lines from DoB() and DoC(), then you've made a huge impact on it.

Put it all together and there is rarely a a simple answer to the "should I split code" question. The only time there is an unambiguous "yes" when when you reduce both code length and communication.

Like any rule, there are exceptions. Mostly I agree with 80x24, but I wouldn't implement it in a linter, because I need to break it every now and again (and Big Ol' Switch Statements are a common cause of breakage).

But as my peer alludes, there are a lot of people who happily write a lot of functions full of side effects and string them together. And in those hands a great mess can be made. I used to be more tolerant of this sort of code but I’ve heard enough complaints to scrutinize this pattern and it does seem to result in quite a few more errors.

There could be a + to the left and maybe a red box around the fake inline-ing.

Although I’ve also found that memorizing the advanced navigation features of the IDE is a pretty good way to get around without upsetting your working memory.

In gnome-shell-mode [1] we can evaluate arbitrary expression and paste the content (virtually) into the buffer. Since javascript functions evaluate to their source it makes for a poor-mans peek-definition: https://imgur.com/a/7Ot7KFy

It would maybe (sometimes) be nice to even intelligently "macro expanded" the call:

    function helper(a, b) {
      let c = a + b
      return a + c
    }
    
    function p(y) {
      let x = helper(1+y, 2)
    }
    
    --> 
    
    function p(y) {
      // inlined x = helper(1+y, 2)
      let x
      {    
        let c = (1+y) + 2
        x = (1+y) + c
      }
    }

[1]: https://github.com/paperwm/gnome-shell-mode

   DoA();

   // A
   ...

API's are hard to get right. API's create dependencies. Most internal API's are hot garbage.

  DoAll() {
    DoA();
    DoB();

    DoA() { ... }
    DoB() { ... }
  }

I have a theory that SRP is the driver behind a lot of this, and that the way it's commonly understood is incorrect (in part because the idea itself is somewhat ambiguous and possibly incoherent). I'll try to go into more detail in a blog post.

I also noticed, during code reviews, that people read and digest statements that are broken up into multiple lines much more carefully than they do compound statements that live on a single line. Leaving me to think that line width is not just an aesthetic matter. It's really a very impactful decision that can have an important, if subtle, influence on overall software quality.

It very much depends on the screen. With a 32 inch screen you can fit three 120-column views at the equivalent of 100 DPI at default zoom. Or even better, six 120x40 views.

Of course I just got stuck writing jupyter notebooks for a month so I’m against all the scrolling crud.