He is absolutely correct in this. However, he's wrong with regards to the level of abstraction. Those "operations" should be part of functions that could be scattered all over the code base in whatever order they were written. But at the end of the day, they will be called sequentially right next to each other.
I've often found this to be the case. The developers I see that make these "god" functions are unable to write and compose their code in layers. They instead see the entire run (start->finish) of their programs as one giant series of "sequential" "operations". So what ends up happening is they've got high-level code, interspersed with low level io/networking/db calls.
At work, I have a 450 line function. What it does is very simple:
1) validate the request; if it's invalid, send back an error. 2) look up some information. 3) send that information.
Step 1 is around 300 lines. But a majority of the lines are huge comment blocks describing what the check is for and referencing various RFCs. Yeah, I could have broken this part off into its own function, but ... there will only be one caller and doing so would be doing so for its own sake [1].
Step 2 is one line of code (it calls a function for the information).
Step 3 is the remainder; it's just constructing the response and sending it.
The code is straightforward. It's easy to follow. It's long because of the commentary.
[1] I did have to deal with a code base where the developer went way overboard with functions, most of which only have one caller. It's so bad that the main loop of the program is five calling levels deep. I once traced one particular code path down a further ten levels of function calls (through half a dozen files) before it did actual work.
When I confronted the developer about that, he just simply stated, "the compiler will inline all that anyway." And yes, the compiler we're using can do inlining among multiple object files, which makes it a real pain to debug when all you have is a core file.
There is benefit in being able to group the chunks, and maybe your comments are quite obvious in the grouping. But if I know I'm looking for a "validation" bug, and I come across a grouping of three functions which are called, say, "PrepareRFCRequest", "GetResponseInformation" and "ConstructResponse" then I can very easily deduce that they're not related to my problem and ignore them. You could say they're not my concern.
A lot of these things I talk about build upon each other. i.e. If you then go ahead and put some sort of input validation inside of your "GetResponseInformation" function, then the grouping / function abstraction is pretty much useless and may even be detrimental when debugging, e.g. in my example above.
"there will only be one caller and doing so would be doing so for its own sake" No, there is benefit to putting it in its own function. Even if there is one caller. Because creating functions isn't always about reducing duplication, but of concerns/abstraction and to me above all composability.
"[1] I did have to deal with a code base where the developer went way overboard with functions, most of which only have one caller. It's so bad that the main loop of the program is five calling levels deep. I once traced one particular code path down a further ten levels of function calls (through half a dozen files) before it did actual work."
Perhaps you, I, and the developer you speak of have a different view of what the "main body/loop" of the program, or any program in general.
It almost sounds like you're talking about http://canonical.org/~kragen/sw/dev3/server.s! Or for that matter http://canonical.org/~kragen/sw/aspmisc/my-very-first-raytra...! But it's unlikely you've had to maintain either of them. I intentionally broke things up like that with the idea that it would make it easier to understand. In fact, I originally wrote httpdito as straight-line code and only later factored it into macros to make it easier to understand.
It sounds like you're saying that this was misguided and I should have just used nesting. I'll have to think about that for a while.
Things like database reads/writes should be refactored out immediately. This is boring stuff that only challenges inexperienced developers. Most developers with some experience can grok the idea of a function named 'SaveToDB(stringthing)' or something like it. When not dealing with common operations, the key here is repackaging your functions in a way that the meaning is significantly conveyed through the name of that function without the name being excessively long. Short function bodies also ensure that things remain quickly absorb-able.
Taking if-blocks or loops and putting them into their own function just to shrink the size of the god function to pretend you're refactoring really serves no purpose though (IMO). This is especially true if the number of times those functions are called is less than 2. (believe me, I've seen it!)
They should be refactored out, yes, but refactored out in a different component with little (ideally none) logic in it, not a different location of the higher-level component.
> Taking if-blocks or loops and putting them into their own function just to shrink the size of the god function to pretend you're refactoring really serves no purpose though (IMO). This is especially true if the number of times those functions are called is less than 2.
Of course it serves a purpose. Take a codebase where you have functions that go over 5 or 10 screens. Now, take a codebase which does the exact same thing, but where the god function is split into smaller and smaller functions depending on the complexity. The second codebase is much easier to read (as long as function names are well chosen). It also means that most, if not all, of the comments necessary to understand codebase 1 (which may or may not be present, and may or may not be up to date) can be removed. The number of call sites for a function does not matter. What matters is how small functions make the code easy to read.
I guess I'm different to you in that regard. To me, my "brain dump" is a bunch of function skeletons that I write out at a high-level. E.g. if I know at some point I'll have to parse a file, I brain dump it as:
result = parse_file(get_file_data())
At that point, I've already defined my "higher-level" brain dump (as you call it) without having to worry about stuff like file-format, filenames, checking for deleted files, etc.
"Taking if-blocks or loops and putting them into their own function just to shrink the size of the god function to pretend you're refactoring really serves no purpose though (IMO)"
Well, depends on the if-blocks I'd say. 99% of the time, I bet you that those blocks and loops can grouped into logical pieces of work that happen in stages. Claiming that we do it just to "pretend" we're refactoring doesn't really add anything to the conversation.
