I agree too. Another example: I find early returns in functions easier to read than “else” with one “return” at the end. Basically vertically linear code as opposed to unnecessary branches and too much indentation, keeping the code slimmer is healthier!
Someone smart said, "When you've lost something, and finally find it, don't put it there again. Instead put it the first place you looked."
I think that applies to code. When I read something I wrote, if I'm annoyed at how it reads, I try to refactor it to be what I wanted to read, and remember to do it that way in the future.
But sometimes what the reader wants is too much work for the writer, so I don't push that effort beyond what it's worth.
I don’t agree that’s what TDD does. You spent inordinate amounts thinking about how you should want it to be, when you could just write it, find where and what about it you dislike, write it again and have actual good code. Also called WET. You spend less time with better results that way and you gain what OP was talking about in the process.
Do other people not consider comments in PRs anymore? Would someone ok a PR [for a function] with 600 lines? Probably not. Let's not be absurd here, but I fundamentally agree that it's too long already.
Yes, maybe it's the wrong abstraction or it won't match in 6 months because people will start calling Hats Pizzas and the logic will be different. Maybe I'll be dead tomorrow. I don't concern myself with what-ifs over unknowns.
I want correctness and some assurance of it. I don't think writing 50? tests to cover this thing is a good way to spend my time (or someone else's). I'll write the left hand side first. When I'm looking at writing tests, it will become something like the right hand side.
Yes linear code is more readable. That's something to consider, but it's not my primary consideration.
Apologies. I wasn't clear. I meant it's a stretch to approve a function that's 600 lines. It would be very hard to reason about. I have clarified in my original post. Thank you for the feedback +1 to you.
My code revivew tool shows me 3 lines of context around changes, which means the untouched comment won't even be seen unless i think to look for context. The tool also makes it hard to comment on those lines that were not touched but should have been.
I've used many code review tools, all suffered from that. (the only exception wasn't a tool: print out of all the code to physical paper and then we all went into a meeting room to review for several hours - this was the best type of code review for finding things wrong, but it was also extremely expensive so I've only done that once in my life)
It's so ridiculous though! We have to get rid of comments because some random in the future might commit some rubbish? It's their fault! not mine! I'm just documenting what works at the time of publication
You are putting something in there that turns out to be disinformation.
I used to believe as you did, but then I started actually looking at old code.
I stored many examples, but I'll give you one to indicate just how bad most comments are:
//Add 1 to x
x:=x+2;
Now why the original author thought it was important to explain in English what the next line of code was going to do in the programming language I am not sure.
I am sure that someone figured out that it was a mistake and did the least they could possibly do to fix the mistake.
Something like 80% or more of the comments I've seen have been misleading, wrong, or useless.
If you do your own survey of someone else's old code so that your ego doesn't get in the way you will probably find something similar.
Although I did talk with people working at NASA and they had someone on their code reviews who was specifically looking at comments. If you are that disciplined then your code will not be that bad, probably.
The example code is vey simplistic, so of course that linear code is more readable, but the idea doesn’t scale.
I think you have to consider things like reusability and unit-test-ability as well, and having all your code in a single function can make reasoning about it more difficult due to all the local variables in scope that you need to consider as possibly (maybe or maybe not) relevant to the block of code you’re reading.
That being said, when I look back on my younger, less experienced days, I often fell into the trap of over-refactoring perfectly fine linear code into something more modular, yet less maintainable due to all the jumping around. There is something to be said for leaving the code as you initially wrote it, because it is closer to how your mind was thinking at the time, and how a readers mind will also probably be interpreting the code as well. When you over-refactor, that can be lost.
So I guess in summary, this is one of those “programming is a craft” things, where experience helps you determine what is right in a situation.
So where's the proof that the function'd code scales? As the complexity of the overall code grows, so would something that gets chopped into dozens of functions to the point of being unreadable.
Suddenly, you realize that the dozens of functions __need to be called in specific orders__, and they are each only ever used once. So really what you're doing is forcing someone to know the magic order these functions are composed in order for them to be of any use.
In a decent programming language you can nest functions, so all the little functions that make up some larger unit of the program are contained within (and can only be called within) that outer function. They serve less as functions to be called and more just as names attached to bits of code. And since they can't be called anywhere else, other people don't need to worry about them unless they're working on that specific part of the program.
Unfortunately organizing your code along the right lines of abstraction is something that just takes skill and can't easily be summarized in the form of "just always do this and your code will be better"
If you organize your code into units that are easy to recompose and remix, well you get huge benefits when you want recompose and remix things.
If you organize your code into units that can't be easily recomposed, then yes you've added complexity for no benefit. But why make units that can't be treated individually?
"As the complexity of the overall code grows, so would something that gets chopped into dozens of functions to the point of being unreadable."
So the answer to this is, "don't chop it into functions in a way that leaves it unreadable, instead chop it into functions in a way that leaves it more readable."
That may be unsatisfying, but it gets to the point that blindly applying rules is not always going to lead to better code. But it doesn't mean that an approach has no value.
There's an easier approach that will also aid you in telling you how to precisely chop up your function.
Simply don't chop up your function until you need a slice of it somewhere else. Then refactor out the bit you need. You'll find out exactly which bits need to be replaced with variables and exactly where the slice needs to happen.
This is the correct answer right here if you have a good enough team. It is still the way I want to work. Unfortunately, I find that there are too many developers who haven't learned that you should always be considering to "refactor as you go". I'm trying to teach by example, but it's an uphill battle.
Exactly. Start with the straight-ahead linear approach and factor out once it's unwieldy.
Same thing for copy pasta funcs -- the first copy is fine, the second one may be too, but after that consider extracting to a parameterized func (a permutation of the Go Proverb "A little copying is better than a little dependency.")
The API shouldn't be that. Expose something easy to use. That is the point of abstractions. It doesn't matter if there are a dozen methods called in order if those dozen methods are called by a helper method, beyond maybe some implementation details.
Really the question should always come up when there are more than say two ways to do things. If I can make a pizza from scratch, reheat a chilled pizza, create a pizza and chill it, reheat a half dozen pizzas, or make three pizzas of the same kind and chill them suddenly the useful abstractions are probably something you can figure out between those helper methods.
Honestly that is the real fear of the left way of thinking. If you add a quantity, whether to cook and whether to chill parameters you end up with a hard API where certain combinations of parameters don't make sense.
Have a clean API and make the implementation as simple as is feasible. Reuse via functions when it makes sense but don't add them willy nilly.
Aka "it is a craft and you figure things out" as someone said in the comments here
> Suddenly, you realize that the dozens of functions __need to be called in specific orders__, and they are each only ever used once. So really what you're doing is forcing someone to know the magic order these functions are composed in order for them to be of any use.
That's where nested functions show their true utility. You get short linear logic because everything is in functions, but the functions are all local scope so you get to modify local scope with them, and because the functions are all named, it is easy to determine what is going on.
If you have dozens of functions that need to be called in specific orders, design and use a state machine and then use a dispatch function that orchestrates the state machine.
I'm very dubious of anyone resorting to "readability" as a justification.
What you're doing by breaking things into functions is trying to prevent it's eventual growth into a bug infested behemoth. In my experience, nearly every case where an area of a code base has become unmaintainable - it generally originates in a large, stateful piece of code that started in this fashion.
Every one who works in said area then usually has the option of either a) making it worse by adding another block to tweak it's behaviour, or b) start splitting it up and hope they don't break stuff.
I don't want to see the "how" every time I need to understand the "what". In fact, that is going to force me to parse extraneous detail, possibly for hundreds of lines, until I find the bit that actually needs to be changed.
> What you're doing by breaking things into functions is trying to prevent it's eventual growth into a bug infested behemoth
Not every piece of code grows into a bug-infested behemoth. A lot of code doesn't grow for years. We're biased to think that every piece of code needs to "scale", but the reality is that most of it doesn't.
Instead of trying to fix issues in advance you should build a culture where issues are identified and fixed as they come up.
This piece of code will be a pain to maintain when the team gets bigger? So fix it when it actually gets bigger. Create space for engineers to talk about their pains and give them time to address those. Don't assume you know all their future pains and fix them in advance.
> In my experience, nearly every case where an area of a code base has become unmaintainable - it generally originates in a large, stateful piece of code that started in this fashion
In my experience it gets even worse with tons of prematurely-abstracted functions. Identifying and fixing large blocks of code that are hard to maintain is way easier that identifying and fixing premature abstractions. If you have to choose between the two (and you typically do), you should always choose large blocks of code.
The great thing about big blocks of code is that their flaws are so obvious. Which means they are easy to fix when the time comes. The skill every team desperately needs is identifying when the time comes, not writing code that scales from scratch (which is simply impossible).
> The example code is vey simplistic, so of course that linear code is more readable, but the idea doesn’t scale.
One of the best reviewed functions I wrote at work is a 2000 line monster with 9 separate variable scopes (stages) written in a linear style. It had one purpose and one purpose only. It was supposed to convert from some individual html pages used in one corner of our app on one platform into a carousell that faked the native feel of another platform. We only needed that in one place and the whole process was incredibly specific to that platform and that corner of the app.
You could argue that every one of those 9 scopes could be a separate function, but then devs would be tempted to reuse them. Yet, each step had subtle assumptions about what happened before. The moment we would have spent effort to make them distinct functions we would have had to recheck our assumptions, generalize, verify that methods work on their own... For code that's barely ever needed elsewhere. We even had some code that was similar to some of the middle parta of the process... But just slightly didn't fit here. Changing that code caused other aspects of our software to fail.
The method was not any less debuggable, it still had end to end tests, none of the intermediate steps leaked state outside of the function. In fact 2 other devs contributed fixes over time. It worked really well. Not to mention that it was fast to write.
Linear code scales well and solves problems. You don't always want that but it sure as hell makes life easier in more contexts than you'd expect.
Note. Initial reactions to the 2000 line monster were not positive. But, spend 5 minutes with the function, and yeah... You couldn't really find practical flaws, just fears that didn't really manifest once you had a couple tests for it.
I worked with an engineer that wrote the most clear and elegant linear code. It was remarkable, never seen anything like it since. I can't reproduce it but I do have an idea of what a well designed linear function looks like.. a story.
I will agree that it takes some skill, not that I am great at it. It's a different kind of skill than abstraction. Reading error handling in c code offered good insights for me to learn linearity better (c code that uses goto to jump to the end of a function for cleanup when an error occurs, for example).
However, if you screw up linear code, you screw up locally. If you write poor small functions, the rest of the team screws up because they barely ever read the contents of your functions that call other functions that call other functions. I've had way more problems with stuff being called slightly out of order, than with large functions.
I was just thinking that if I _needed_ to refactor this I might structure the stages as chapters in a book. One might be able to write an inner class or some such that had a “table of contents” function that called each stage in sequence as a void function with data managed out of line, maybe via cleverly designed singleton structs. Then the code itself can be written in order with minimal boilerplate between stage boundaries.
I think I’ve worked with some Python that looked and worked this way. I can’t place the details but probably in a processor pipeline running over a particularly hairy data format. Consider ancient specifications written by engineers talking on the phone encapsulated in relatively “modern” but still vintage specifications, sometimes involving screen-scraping a green screen mainframe terminal, wrapped in XML and sent over the internet. Anyway, point is I couldn’t agree more about stories.
I don't think the fact that a function works well is a good enough reason to write a 2000 line function. Sometimes there are long pieces of code that implement complex algorithms that are difficult to break into smaller pieces of code, but those cases are limited to the few you mentioned.
Computers execute code in a linear fashion, why on earth would you "need a reason" to NOT abstract something? Just because abstraction is often the right thing to do doesn't make it the base case.
It's like saying you need a reason not to add 4000 random jumps in your assembly code just to make it more difficult to read...
I guess all we know is there were 2K lines of code and the commenter thinks that was the right way to do it. It would be necessary to see the code to appropriately critique it.
Not just the commenter, but his team as well. It passed code review with flying colors, apparently. The moral of the story is that there always exceptions and developers should not be ideologically committed to one approach above all else.
we know more than that: You could argue that every one of those 9 scopes could be a separate function, but then devs would be tempted to reuse them. Yet, each step had subtle assumptions about what happened before.
what we don't know is if it would have been possible to abstract those assumptions away so that functions could have been defined without them.
> that implement complex algorithms that are difficult to break into smaller pieces of code
My longest code is always image processing. It's usually too hard to break up for the sake of breaking up. There's nothing to reuse between the calls to filters/whatever.
I don't know if it is still like this, but the code for dpkg used to be like this, and it was amazing: if you ever needed to know in exactly what order various side effects of installing a package happened in, you could just scroll through the one function and it was obvious.
To this end, I'd say it is important to be working in a language that avoids messing up the logic with boiler plate, or building some kind of mechanism (as dpkg did) to ease error handling and shove it out of the main flow; this is where the happy path shines: when it reads like a specification.
Isn’t that the fucking point? Having a 2000 line function is a code smell so bad, I don’t care how well the function works. It’s an automatic review fail in my book. Abstractions, closures, scope, and most importantly - docs to make sure others use your functions the way you intended them. Jesus.
Agreed. I’ve written plenty of software of all kinds and have never had to write a 2000 line long methods (although I have had the joy of refactoring such messeses a time or two).
Just don’t do that. Your code doesn’t have to have abstractions out the wazzo, but if your class (or method) is getting bigger than 1000 lines that’s a great sign that it’s doing too much and abstractions can be teased out. Your future self will thank you, as well as your team.
> You can't create the right abstraction until you fully understand the code, but the existence of the wrong abstraction may prevent you from ever doing so. This suggests that you should not reach for abstractions, but instead, you should resist them until they absolutely insist upon being created.
At least in the mobile world, I find that this “no abstraction” approach is the default one, and it usually leads to huge objects which do everything from drawing views to making network requests to interacting with the file system. These kinds of classes are quite hard to work in, hard to test, and also keep snowballing to get bigger and bigger. Things usually end with unmaintainable code and a full rewrite.
I am not saying you need to create complex abstract hiarchies right off the bat. But usually, it’s pretty easy to tease out a couple significant abstractions that are very obvious, and break down your classes by a factor of two or three. Just getting such low hanging fruit will prevent you from ever having a 2000 line long method.
And for the folks who are saying that they make sure to not add abstractions too early - are you disciplined enough to go back and add them later? I feel like if you’re the kind of engineer that busts out 2000 line methods, you’re also not going to refactor it as this method grows to 2500 or 3000 lines or beyond.
Probably most robust software you depend on is full of solid, quality abstractions. Learning to write code like this takes practice. The wrong abstraction might be wrong, but it’s one step closer on your journey to growing as an engineer. You won’t grow if you never try.
Some devs did find it a code smell... But each scope had a clear short high level comment describing what it did, there were end to end tests for the method, and very little state flowed from scope to scope (some did) - because that's what scoprs do... Prevent variables from leaking.
My point is the code smell isn't always accurate, and there are times and even for 2000 line monsters other devs agreed that it was the best way to hide complexity away from the rest of the codebase in that case. If we ever needed to factor things out (we never did), we could spend some effort and do it.
Definitely not that many. Even for me this was an outlier, but it made me more comfortable with functions most people would consider long.
I'd like to clarify this was not necessarily 2kloc-complex, this was just 2kloc-long-and-not-really-meant-to-be-reused. It was a fairly long but linear process that was out of the ordinary for the rest of the codebase. It could easily have been split (hell, I had 9 fairly separate stages), but calling any of the intermediate stages out of order or without the context of the rest of the execution flow... would have been a foot gun for someone else. And, as time showed, we never needed those stages for anything else.
>The moment we would have spent effort to make them distinct functions we would have had to recheck our assumptions, generalize, verify that methods work on their own
Why? Why can't the functions say "to be used by <this other function>, makes assumptions based on that function, do not use externally"? Breaking out code into a function so that the place it came from is easier to maintain... does not mandate that the code broken out needs to be "general purpose".
Specifically, in that place, there was no need. And prematurely splitting it would have caused us to overthink and over generalize. Having a long, linear and tested function was a better choice.
I understand your point, but perhaps that would have simply been an opportunity to refine your approach to code design. If such a situation leads to excessive deliberation and overgeneralisation, your code base must be riddled with unnecessary overthinking and overgeneralisation.
I have been programming for more than 30 years. Except for code generated explicitly to be only consumed by machine, I've never come across a function consisting of 2000 lines of code that should not have been broken up. Something is wrong there, and if you show me the code, I'll tell you what's wrong with it.
At first I thought how horrible, but basically you have sort of 9 functions within the same scope, each having a docstring. So I guess not too different from splitting them up.
I read you have "end to end" tests.
One question though: Wouldn't each part benefit for having their own unit tests?
Maybe, maybe not. For our particular case it would have been mostly wasted effort.
I found that I like to write tests at the level of abstraction I want to keep an implementation stable. I'd be totally fine if someone went in and changed the implementation details of that long process if needed. We cared that stuff got cleaned up at the end of the process, that the output matched certain criteria, that certain user interaction was triggered and so on... In that case it made more sense to test all our expectations for a larger scope of code, rather than "fix" the implementation details.
Tests usually "fix" expectations so they don't change from build to build. Tests don't ensure correctness, they ensure stuff doesn't alter unexpectedly.
Tests effectively freeze requirements; you should test those things which should be preserved throughout any changes, and not test those things which should be open to change. In this case, it seems that is no real requirements for any of these 9 steps - perhaps the implementer could figure out how to do the same outcome by skipping a step or merging two steps, and the existence of unit tests for these 9 functions somehow encodes the idea that these 9 functions each are inherently needed, which is not necessarily true.
If the sub-functions could be reused and people would be tempted to change them, then that’s what your tests are for. In fact, it’s often tricky to test the sun-function logic without pulling them out because to write the test you have to figure out how to trick the outer function to get into certain states. Follow the Beyoncé rule: if you like it: put a test on it. Otherwise it’s on you if someone breaks it.
> this is one of those “programming is a craft” things, where experience helps you determine what is right in a situation.
You are right here.
The key insight on why giant linear functions are often more readable (and desirable) is because they allow you to keep more concepts/relationships simultaneously together as a single chunk without context switching which seems to aid our comprehension. An extreme proponent is Arthur Whitney (inventor of the K language) who writes very terse (almost incomprehensible to others) code so as to accommodate as much as possible in a single screen.
Two examples from my own experience;
1) I found reading/understanding/debugging a very large Windows message handler function (i.e. a WndProc with a giant switch statement containing all the business logic) far easier than the same application rewritten in Visual C++ where the message handlers were broken out into separate functions.
2) The sample code for a microcontroller showed an ADC usage example in two different ways; One with everything in the same file and another where the code was distributed across files eg. main.c/config.c/interrupts.c/timer.c/etc. Even though the LOC was <200 i found the second example hard to understand simply because of the context switch involved.
YEAH, but the moral that should be taken from that is not "it's always better to write huge, linear functions". Rather, "there are cases where huge, linear functions make sense because of the way the code needs to interact with things". Along the same lines, there are cases where breaking the code up into smaller functions, and calling them from the main function, makes more sense".
Not quite what i was trying to convey. Linear code actually has a lower burden on one's cognitive load and thus easier to comprehend. But of course it breaks down at certain sizes (variable) which is when it makes sense to partition it into smaller pieces and apply Abstraction etc.
> Linear code actually has a lower burden on one's cognitive load and thus easier to comprehend. But of course it breaks down at certain sizes ...
I agree with this, but I think the combination of those two sentences winds up being "linear code is sometimes easier to comprehend, and sometimes not". The statement "linear code is easier to comprehend" is misleading. Your statement makes it seem like "at certain sizes" is the edge case; whereas, in my opinion, it's the only case that really matters. For a small enough block of code, "easier to comprehend" becomes a moot point.
> See for example Cyclomatic Complexity
I think that's only tangentially related. Cyclomatic Complexity deals with branching, which is somewhat orthogonal to refactoring out code to separate functions (though refactoring can make the branching easier to read, since it shows more in a smaller area).
> an extreme proponent is Arthur Whitney (inventor of the K language) who writes very terse (almost incomprehensible to others) code
But k has a small set of built-in commands and a built-in database; it was made for fast analysis of stock information, so with that you have everything you need and you use the same semantics. The only thing you need to know is the data structure and you can build whatever you need.
So in this way, it's very likely that, given two tables A + B and 'bunch of operations' X on A and 'bunch of operations Y' on B where Y depends on the result of X, and given the tasks to;
- create X' = X
- create XY' = X + Y
to implement XY without knowing X already exists rather than figure out X exists and reuse it.
The problem with not k (or programs written in similar style; it doesn't really matter what the programming language is), that we have learned to use the second style from the article, and, more extreme, to separate everything out in layers. You cannot even reach the data model without going through a layer (or more) of abstractions which makes it necessary not only to know the datamodel in detail but also find the matching findXinAandApplyWithYToB(). Where X & Y & A & B are often some kind of ambiguous and badly named entities. And then there is of course badly designed databases which is also quite the norm as far as we see, so there is a much lower data integrity which means that if you create something without checking all the code that touches it, that you might change something and the data becomes inconsistent.
I notice the same when working on systems built with stored procedures on MSSQL/Postgres; it is far quicker to oversee and (at least basically) understand the datamodel (even with 1000+ tables, which is rather normal for systems we work with) than it is to understand even a fraction of a, let's say Go, codebase. So when asked to do do a task XY', you are usually just not searching for X'; you are simply reading the data used in X & Y and whop up a procedure/query/whatever yourself. It's simply much faster as you have a restricted work surface; the model and sql (I know, you can use almost any language in postgres, but let's not here) and you can reason about them and the tasks at hand when you shut off internet and just use your sql workbench.
Nice post. If i understand you correctly; you are saying that K is specialized enough (my knowledge is only cursory here) that you can directly work with the data model easily rather than going through multiple layers of abstractions and hence linear code is normal. In other languages it may not be so easy to do and multiple layers of abstractions only make things harder to comprehend. True, IMO Abstraction should always follow Understanding of the Problem space and not some arbitrary dogma. What i find infuriating nowadays is "cargo culting" where people blindly follow something because they read it somewhere/listened to somebody without thinking through the motivations involved and whether it is applicable to their current problem. In other words "they don't think" for themselves. Examples are "OO is bad"(it is not), Agile/Scrum processes will magically solve all your PM problems(hell, no!), Using the latest library/framework/API/fad will magically make your system better (no!), etc. etc.
> If i understand you correctly; you are saying that K is specialized enough (my knowledge is only cursory here) that you can directly work with the data model easily rather than going through multiple layers of abstractions and hence linear code is normal.
Yes, it is often just easier to write the linear code than figure out if you can reuse anything because the space is small. I think a good 'feeling' for this is, if you need internet search/package managers/copilot etc for something or if you can just write working code sitting on a desert island, quite possibly on paper. For instance, for C, asm (arm/68k/z80/8080 and older intel), k and some others I can write working code like that for non-toy applications in specific domains. And, at least for me, those languages lend themselves very well for this linear programming. Incidentally, but not related, this is for me also the most enjoyable way of programming; I kind of really hate using libraries. That's also because we work in heavy regulatory areas where you cannot just add them; you have to inspect them and sign of on them and, of course, most of them are terrible...
> The key insight on why giant linear functions are often more readable (and desirable) is because they allow you to keep more concepts/relationships simultaneously together as a single chunk without context switching which seems to aid our comprehension.
The problem with giant linear functions is that those concepts get separated by sometimes thousands of lines. Separating out the high-level concepts vs the nitty-gritty details, putting the latter in functions that then get called to implement the high-level concepts, does in my experience in most cases a better job of keeping related things together.
It is "Mechanism" which should be separated out and encapsulated while "Policy" (aka business logic) is what is better centralized as a linear (possibly large) function.
I have seen many instances where people just out of habbit factor out a lot of linear code that will never be reused into separate functions.
These pieces of code then often end up being private functions of a class. With state. Since they are private functions now, they are not really testable.
So now we got a lot of private functions that are only called once and typically modify side effect state. When these functions are grouped together with the caller, it is actually still a bit readable in simple cases.
But then after a while someone adds other functions in between the calling function and the factored out ones.
Now we have bits and pieces modifying different side effect state that no one knows if they are called from different places without getting a call graph or doing a search in the class file.
If you insist on making the code non-linear, I'd beg you to at least consider making these factored out private funcs inner funcs of the calling function if your language supports that. This makes it clear that these functions won't be called from anywhere else.
As with so many things in life, in a real codebase this is not an either/or, but an art of combining the two into something that stays readable and maintainable.
If the function was truly linear having a long function wouldn't be so bad. But it actually isn't, the example contains multiple branches!
Will people bother testing all of them? Or will they write a single test, pass in a pizza and just glance at it actually working? My guess is the latter, as testing multiple branches from outside is often tedious, vs testing smaller specialized functions.
> The example code is vey simplistic, so of course that linear code is more readable, but the idea doesn’t scale.
...that's basically why common sense and taste in programming is still required, it's not a purely mechanical task. That's also why I'm not entirely a fan of automatic code formatting tools, they don't understand the concept of nuance.
What do you mean never write any tests? The api should be the same. Order goes in, pizza comes out. The rest is implementation details that should not be exposed to a test.
you're right, the api for ordering a pizza will probably stay the same.
the cooking process won't though. stuffed crust? add some stuff in the middle. square? add some stuff in the middle. deep dish? add some stuff in the middle.
iterate a while and your "one golden test" is what falls down.
Those items are all testable through the createPizza method. There should be lots of tests! You've made up the one golden test scenario as a strawman. Every scenario you listed changes the expected output(the pizza). If you are testing internal methods, your tests are going to tell you you have broken, even if the pizzas created are 100% correct. So people won't clean the code, because the tests break, and they don't know if they are actually broken.
Refactor when those things are needed, right now the cooking process is stick it in a warm over for x minutes.
What are you testing there?
The oven was preheated? Put in an assert, that doesn't need a test.
That it stayed in for x minutes? You assuming the builtin sleep function is broken? Don't test library code, that's not your job.
That the oven actually preheated correctly, that was discussed in the article, the oven and it's preheat method should be a dependency that gets passed in, again not needed to be tested here.
Also in your example you are testing whether an if condition was evaluated as true.
Give me an example of a stuffed crust pizza cooking process that has a unit test which cannot be checked by looking at the resulting pizza.
The goal of this unit of code is to create a pizza from an order.
There are no observable side affects.
To verify the correctness of this unit of code, the only requirement is to map all inputs to outputs.
Any assertions other than that mapping are not requirements to test correctness.
Yes, and I can create an even larger 'unit' of code around it, declare that it has 'no observable side effects', and claim that `createPizza` is just an implementation detail of that, and therefore needs no testing.
Maybe? We don't know about the other code. Most code I work with would be sticking this output in a db or email or something, so any larger piece can't be isolated, so is no longer a single unit.
It's about isolation from side affects, not how much code you can stick behind a single method call.
Mixing different levels of abstraction makes the code harder to understand. Linear code is probably good because the examples in the body are simple. It's one thing to separate code into separate files, but it's another to break up code snippets in one file.
Hard agree. And I used to belong to the other camp.
The basic tension here is between locality [0], on the one hand, and the desire to clearly show the high-level "table of contents" view on the other. Locality is more important for readable code. As the article notes, the TOC view can be made clear enough with section comments.
There is another, even more important, reason to prefer the linear code: It is much easier to navigate a codebase writ large when the "chunks" (functions / classes / whatever your language mandates) roughly correspond to business use-cases. Otherwise your search space gets too big, and you have to "reconstruct" the whole from the pieces yourself. The code's structure should do that for you.
If a bunch of "stuff" is all related to one thing (signup, or purchase, or whatever), let it be one thing in the code. It will be much easier to find and change things. Only break it down into sub-functions when re-use requires it. Don't do it solely for the sake of organization.
> Only break it down into sub-functions when re-use requires it. Don't do it solely for the sake of organization
What about for testing? What about for reducing state you need to keep in mind? What about releasing resources? What about understanding the impact of a change? Etc.
Consider an end of day process with 10 non-reusable steps that must run in order and each step is 100 lines. Each step uses similar data to the step before it so variables are similar but not the same. You would really choose a 1000 line single function?
For "use-case" code like this with many steps, you are typically testing how things wire together, and so will either be injecting mocks to unit test, in which case it is not a problem, or wanting to integration or e2e test, in which case it is also not a problem.
If complex, purely logical computation is part of the larger function, and you can pull that part out into a pure function which can be easily unit tested without mocks, that is indeed a valid factoring which I support, and an exception to the general rule.
> What about for reducing state you need to keep in mind?
Typically not a problem because if the function corresponds to a business use-case, you and everybody else is already thinking about it as "one thing".
> What about releasing resources?
Not a problem I have ever once run into with backend programming in garbage collected languages. Obviously if you are in a different situation, YMMV.
> Consider an end of day process with 30 non-reusable steps that must run in order and each step is 100 lines.
I would use my judgement and might break it down. Again, I have never encountered such a situation in many years of programming.
You seem to be trying to find the (ime) rare exceptions as if those disprove the general rule. But in practice the "explode your holistic function unnecessarily into 10 parts" is a much more common error than taking "don't break it down" too far.
let DebugFlags = {StepOne=false, StepTwo=false, StepThree=true};
if (DebugFlags.StepOne) { ... }
if (DebugFlags.StepTwo) { ... }
if (DebugFlags.StepThree) { ... }
Your training in structured, DRY and OOP will recoil at this: More branches! Impossible. But your spec says "must run in order". It does this by design. Every resource can be tracked by reading it top to bottom, and the only way in which you can miss it is through a loop, which you can also aim to minimize usage of. The spec also says "uses similar data to the step before it". If variables are similar-not-same, enclose them in curly braces so that you get some scope guarding. The debug flags contain the information needed to generate whatever test data is necessary. They can alternately be organized as enumerated state instead of booleans: {All, TestOne, TestTwo, TestThree}.
Long, bespoke linear sequences can be hairy, but the tools to deal with them are present in current production languages without atomizing the code into tiny functions. Occasionally you can find a useful pattern that does call for a new function, and do a "harvest" on the code and get its size down. But you have to be patient with it before you have a good sense of where a new parameterized function gets the right effect, and where inlining and flagging an existing one will do better.
I went the opposite direction: I used to be in the linear code camp, and now I'm in the "more functions" camp.
For me the biggest reason is state. The longer the function, the wider the scope of the local variables. Any code anywhere in the function can mutate any of the variables, and it's not immediately clear what the data flow is. More functions help scopes stay small, and data flow is more explicit.
A side benefit is that "more functions" helps keep indentation down.
At the same time, I don't like functions that are too small, otherwise it's hard to find out where any actual work gets done.
The better solution to this is to use nested functions that are immediately called, rather than top level functions. That lets you cordon off chunks of state while still keeping a linear order of definition and execution. And you don't have to worry about inadvertently increasing your API maintenance burden because people started to depend on those top level functions later.
> Any code anywhere in the function can mutate any of the variables
Regardless of the language I'm using, I never mutate values. Counters in loops or some other hyper-local variables (for performance) might be the inconsequential exceptions to this rule.
> More functions help scopes stay small, and data flow is more explicit.
Just write your big function with local scope sections, if needed (another local exception to the rule above). Eg, in JS:
let sectionReturnVal
{
// stuff that sets sectionReturnVal
}
or even use IIFE to return the value and then you can use a const. "A function, you're cheating!" you might say, but my goal is not to avoid a particular language construct, but to maintain locality, and avoid unnecessary names and jumping around.
> A side benefit is that "more functions" helps keep indentation down.
It is also worth noting that solving this problem with function extraction can often be a merely aesthetic improvement. That is, you will still need to keep hold the surrounding context (if not the state) in your head when reading the function to understand the whole picture, and the extraction makes that harder.
Using early returns correctly, by contrast, can actually alleviate working memory issues, since you can dismiss everything above as "handling validation and errors". That is, even though technically, no matter what you do, you are spidering down the branches of control flow, and therefore in some very specific context, the code organization can affect how much attention you need to pay to that context.
> I don't like functions that are too small, otherwise it's hard to find out where any actual work gets done.
Precisely, just take this thinking to its logical conclusion. You can (mostly) have your cake and eat it too.
It is also super powerful technique when using the closure of a function as a way to encapsulate logic and state. A good example is this implementation of a json parser in js[1]. Attempt at lifting the lexer functions or state out of the function would result in every function needing to be wrapped with a factory function. Parser have always been tricky and before I knew this technique I would have reached for a parser combinator/generator but this is a very sensible way of doing it.
They both read linearly. In the version with smaller functions taken out, there's a table of contents at the top of the page and it summarizes the dataflow between the steps. It seems like an appealing read order, assuming you're going to read the whole thing.
For it to stay this readable, though, you'd need to move the functions around if you change the order of the steps. And that's fine if they're private functions, called only from the table of contents. Only, nothing forces you to keep them in order, or even to think about how it reads overall.
It often happens that functions start being reused in a way that can't be linearized anymore. Sometimes people give up and sort them alphabetically, or it's just random.
I agree that placing sequentially executed code in order of execution often improves readability over abstracted code (especially dynamic dispatch and static/dynamic traits). A similar article is at http://number-none.com/blow/john_carmack_on_inlined_code.htm..., but linear code has its own failure modes, if code is not factored into blocks with identifiable functionality and constrained/documented side effects (for example 500-line functions twiddling hardware registers and reading/writing global variables). Carmack later wrote an article in support of small-f functional programming and avoiding side effects and global state when practical (https://web.archive.org/web/20190123060017/http://gamasutra...., the article lost all line breaks during migration to gamedeveloper.com)
Another article that touches on this idea (among others) is https://loup-vaillant.fr/articles/source-of-readability which advocates that "code that is read together should be written together" (reading it made me confused until I realized it meant "placed together"), specifically "Consider inlining functions that are used only once".
I find Linear B more readable than Linear A, but I agree with the OP, if there were additional explanatory comments in Linear A code, then it would be probably more readable than Linear B.
Pass in an order, assert the pizza that comes out is correct.
The entire function is a unit which can fit on my phone screen and has no external dependencies other than possibly oven, which was discussed in the article, it should probably have been passed in, aka dependency injection.
Since the example was golang, I personally love uberFX to define modules and dependencies between modules. When you do it that way, unit tests become really easy.
It isn't necessary with golang to do this at all, but it really helps build consistent structure throughout the entire app, so I do it.
Speaking from personal experience. I built a small golang process that ran on around 25k worker machines. It had to be bug free cause if it crashed and stopped running, it meant updating a whole lot of computers across multiple data centers, by hand.
We unit tested everything and the project worked out really well because of that.
unit test is overrated because most of the problems can be solved via correct by construction methods. Like, do you really need to check if this "kind" variable is equal to "Veg"? This could have easily been solved by using Enum. Similarly, global or not can be solved by using classes/structs that don't have any constructors or something like that.
Functions should exist at the level of concepts:
1. arr | flat | map | collect as HashMap makes sense.
Agree. Furthermore if you are going to split it, you want to make sure no one boxes a raw pizza because they added a bad switch statement or something, as that would be clearly bad. So if you want a split version than you should use types to mark the transitions between states.
boxPizza should take a CookedPizza, and BakePizza should take a RawPizza and return a CookedPizza etc.
Both functions take an order and return a pizza. This seem perfect for testing.
Why would you care where the oven comes from, if the function deliver perfectly baked pizza? A unit test should test against the public interface and not be coupled to implementation details, since that will hamper refactorings.
> which make mocking really difficult.
Mocking is an antipattern anyway, and should be avoided except for nondeterministic components like current time or stateful external services.
This post presents why object oriented programming is harder than it looks.
“I’m gonna return a pizza because I want a pizza”
When of course, what one really wants is a pizza in a box. And the oven objection is also kind of funny. It leads to a “but computers are so fast, why can’t they build me a new oven for each pizza?”
People think they want real-world analogies, which they hope will make code easier to reuse and maintain when what they really want are deep modules with clean interfaces, for which object orientation is not necessary in the least.
I wholeheartedly disagree. Linear functions like this promote laziness in variable naming (var a1, c_tfr, bvf, etc). This also leads to buggy side effects such as having multiple nested if statements performing a plenko-machine determination of code branching. It’s horrid. It’s unmaintainable. It guarantees that someone will have to rewrite it after your gone, because you will be gone.
This is the same as someone arguing for scrolls when books with table of contents and appendices are far superior.
Your rant is misplaced; it is the spirit rather than the letter of the thing that matters. Linear giant code is often easier to comprehend for structures like state machines where you can follow the business logic from one stage to another easily.
DRY, SOLID, there’s a wrath of principles on why this isn’t correct. Here’s what Code Complete [0] has to say…
>” From time to time, a complex algorithm will lead to a longer routine, and in those circumstances, the routine should be allowed to grow organically up to 100-200 lines. (A line is a noncomment, nonblank line of source code.) Decades of evidence say that routines of such length are no more error prone than shorter routines. Let issues such as depth of nesting, number of variables, and other complexity-related considerations dictate the length of the routine rather than imposing a length restriction per se.
If you want to write routines longer than about 200 lines, be careful. None of the studies that reported decreased cost, decreased error rates, or both with larger routines distinguished among sizes larger than 200 lines, and you’re bound to run into an upper limit of understandability as you pass 200 lines of code.”
These are all just guidelines/heuristics and should not be treated like inviolable laws. Thus all advice should be adapted to the problem at hand in the service of Readability/Comprehensibility first.
Instead of repeating myself, i point you to my other comments in this thread for details.
this is anecdotal of course, but as someone who has never written a line of production Go code (but can tell at a visual glance this is in fact, Go), small functions (green) made sense to me as soon as I started reading it. The single function code (red) became hard to follow at some point. It felt like the function was doing 10 different things with a lot of branching and no particular single purpose. Maybe it's the Python background in me, but I am not seeing how the single function is better to read than small, self-contained functions.
It's not a Go thing. I've inherited a number of large linear functions of the author's favored style in several C-syntax family languages, they all become increasingly incomprehensible as they grow longer and older. For any advocate of that style, the only way to maintain them (and retain their supposed clarity) is to extract functions and then re-inline those functions after a comprehensive refactoring. Otherwise, they accrue so much cruft over time that their legibility is completely lost.
Your only other option is to freeze them and never make changes, that doesn't happen much in real-world code (though it probably should).
Literally extracting the functions and then re-inlining them makes no sense. Having that as a sort of mental model while you're working on the code does make sense.
It’s to enable refactoring when it grows large. Most effective way I have found for 1k SLOC or larger functions. I usually don’t re-inline because the result after refactoring is almost always clearer.
Trying to in-place refactor those things is an exercise in frustration. That’s part of why they grow so large, from observing their proponents in action. They don’t actually know what the functions do, only where to add a new path and repeat themselves.
Everyone saying "linear code doesn't scale" actually has it backwards - it's concise functions with a deeply nested call stack that really becomes a nightmare in large codebases. It's never obvious where new code should be added, the difficulty of understanding what the effects of your changes will be increases exponentially since you have to trace all the possible ways code can get called, you end up with duplicated subroutines, etc etc.
99% of the time, you haven't actually come up with a good abstraction, so just write some linear code. Prefer copy/pasting to dubious function semantics.
Well you're describing a readability problem. And you're essentially saying readability is what causes it not to scale.
If we consider the concepts orthogonally meaning we don't consider the fact that readability can influence scalability then "everyone" is fully correct. Linear code doesn't scale as well as modular code. The dichotomy is worth knowing and worth considering depending on the situation.
That being said I STILL disagree with you. Small functions do not cause readability issues if those functions are PURE. Meaning they don't touch state. That and you don't inject logic into your code, so explicitly minimize all dependency injection and passing functions to other functions.
Form a pipeline of pure functions passing only data to other functions then it all becomes readable and scalable. You'll much more rarely hit an issue where you have to rewrite your logic because of a design flaw. More often then not by composing pure functions your code becomes like legos. Every refactoring becomes more like re-configuring and recomposing existing primitives.
I disagree. It's not the purity of the functions, its having to know the details of them. The details, which could have existed here, are now in two other places. If you need to figure out how a value is calculated, and you use a half dozen functions to come to that value, you now have a half dozen places you need to jump to within the codebase.
Small functions increase the chances of you having to do this. Larger ones decrease it, but can cause other issues.
Also, many small functions doesn't make code modular. Having well defined, focused interfaces (I don't mean in the OO sense) for people to use makes it modular. Small functions don't necessarily harm it, but if you're not really good at organizing things they definitely can obscure it.
I think you’re right about side effects being the missing ingredient to this discussion, that is leading people to talk past each other. The pattern’s sometimes called “imperative shell, functional core”.
And I totally agree, this is how you write large code bases without making them unmaintainable.
Where to go “linear” vs “modular” is an important design choice, but it’s secondary to the design choice of where to embed state-altering features in your program tree.
I think people dislike modular code because they want to have all the “side-effects” visible in one function. Perhaps they’ve only worked in code bases where people have made poor choices in that regard.
But if you can guarantee and document things like purity, idempotency, etc, you can blissfully ignore implementation details most of the time (i.e. until performance becomes an issue), which is definitionally what allows a codebase to scale.
Another risk is if you add print_table() then someone else is going to find it and use it in their code, but also add a little flag to adjust the output for their use case.
I think we all know at least some functions like this in a code base. All it takes is for a newcomer to come across a complex function that they need to update some logics for but also don't understand it enough to refactor, so they just added some parameters with default values and call it a day.
The issue is that this approach is almost guaranteed to produce basically untestable code with a myriad of invalid/nonsensical/completely broken input combinations, and its impossible to refactor, too, because you don't even know which parts of the parameter space are actually ever needed, or even how they are supposed to interact.
Whenever function semantics need to change, everything degrades further because of refactoring uncertainties (=> you end up with even more parameters).
This will also be extremely resistant to optimization because even finding the "happy path" is non-trivial.
As a fresh dev, I’d like to know the answer to this as well. Abstract to function w multiple params, abstract to multiple functions, no abstraction and keep as switch statement.
`print_table() + print_table_without_emoji()`
vs
`print_table(remove_emoji= False)`
vs
`switch table_name:
case emoji:
print(table)
case no_emoji:
print(table no emoji)`
If callers typically know statically whether they want emoji or not (in other words, if the parameter would typically be a literal true or false at the call site), then the parameterless version is better. (Note that you can still factor out common code from the two functions into a separate private function if you like. So the parameterless version doesn’t necessarily imply code duplication.) If, on the other hand, callers tend to be parameterized themselves on whether to use emojis or not, then the parameterized version is better, because it avoids having to make a case distinction at the call site.
It depends on the implementation, but in general you prefer the parameterless version because in theory a bug that shows up in print_table has less code it could be in (less surface area to debug).
Without understanding the implementation no one can truly say which is the better approach, but this idea of "surface area for bugs" is something that should be considered when approaching these types of decisions.
Honestly? Both reek of SRP violation.
Why should print_table specifically care about emoji?
if needed remove the emoji, then print. if performance/table size is an issue, working via streams/generators/etc. should be on the (heh) table anyway.
But if you have conceded to being in quick&dir^H^Hpragmatic-land anyway, IMO both can be ok depending on the context.
That’s what tests are for. And if `print_table` is factored properly then they won’t want to add flags, they’ll make a new function out of the pieces of `print_table` that has distinct behavior of its own.
Maybe there's a way where I can code for every possible future with minimal effort. I'm talking about a pattern that isn't a form of premature optimization. Just a rule.
Your way of coding is, coding for the most probable future. Distinctly different of coding for every possible future.
How about this: most code has hard chunks or even sections that can be nearly impossible to figure out without a significant time investment. We can skip the intermediate steps and just move straight to a document that explains the architecture so that we may stop trying to jump through hoops to avoid writing non-comment documentation.
The amount of places I’ve worked at that don’t even have accessible DB schemas is mind-boggling.
I feel like there a happy medium between the two, the left can easily be made more simple by factoring out one or two functions however the right went too far.
The prepare and addtoppings functions should be one function, prepare effectively just fills in a struct and calls add toppings, its pointless to seperare them.
The Bake function simply prepares the over for cooking, which the author mentioned should be a dependency with a method and then factors 4 lines of code into a new function for no reason. The bake and bake pizza function should be one function.
You can then keep the box function as is.
That would be both easier to maintain and easier to read.
The box function is broken too. You box the pizza and then return the pizza...but the box is logically a wrapper for the pizza. `box(pizza)` should return a boxed pizza. A box with contents=[pizza]. Maybe some sauce and pepperoncini in there too.
Plus all these functions are impure. Which isn't always bad but if you can prevent things like boxing it before baking it, you should.
And what even... this entire example is just horrendous. You box the pizza and then slice the pizza? Ready = box.Close()? Can the Close() operation fail? And then the pizza is not ready? Why not throw an error, now the caller has to check if the pizza that got returned to them is even ready...? And that fact is even more hidden on the right side. Same for Sliced and Boxed.
This entire function is clearly a factory for a boxed pizza with toppings which is baked.
I'd argue the entire box.Close() method is slideware and wouldn't exist since it likely is just a return true. You can just as easily just say pizza.Ready = true. Reading this code afterwards I would think there was some stupid requirement somewhere for a pizza.Ready property so someone added it and would check a commit log to see if it can just be removed.
Decent catch there though, the box can also be a dependency that get's passed in.
366 comments
[ 2.3 ms ] story [ 398 ms ] threadI think that applies to code. When I read something I wrote, if I'm annoyed at how it reads, I try to refactor it to be what I wanted to read, and remember to do it that way in the future.
But sometimes what the reader wants is too much work for the writer, so I don't push that effort beyond what it's worth.
Edit to add: and in 6 months, instead of being one short page of code, it'll be 600 lines long and impossible to understand or modify safely
Yes, maybe it's the wrong abstraction or it won't match in 6 months because people will start calling Hats Pizzas and the logic will be different. Maybe I'll be dead tomorrow. I don't concern myself with what-ifs over unknowns.
I want correctness and some assurance of it. I don't think writing 50? tests to cover this thing is a good way to spend my time (or someone else's). I'll write the left hand side first. When I'm looking at writing tests, it will become something like the right hand side.
Yes linear code is more readable. That's something to consider, but it's not my primary consideration.
I've used many code review tools, all suffered from that. (the only exception wasn't a tool: print out of all the code to physical paper and then we all went into a meeting room to review for several hours - this was the best type of code review for finding things wrong, but it was also extremely expensive so I've only done that once in my life)
You are putting something in there that turns out to be disinformation.
I used to believe as you did, but then I started actually looking at old code.
I stored many examples, but I'll give you one to indicate just how bad most comments are:
//Add 1 to x x:=x+2;
Now why the original author thought it was important to explain in English what the next line of code was going to do in the programming language I am not sure.
I am sure that someone figured out that it was a mistake and did the least they could possibly do to fix the mistake.
Something like 80% or more of the comments I've seen have been misleading, wrong, or useless.
If you do your own survey of someone else's old code so that your ego doesn't get in the way you will probably find something similar.
Although I did talk with people working at NASA and they had someone on their code reviews who was specifically looking at comments. If you are that disciplined then your code will not be that bad, probably.
This is to avoid the introduction of bugs by tempting someone to refactor or "fix" your code.
A properly written code, with good naming conventions for parameters, variables and functions should be easy to understand without comments.
I should not need anyone to tell me the next few lines will
//Heat oven
That should be self explanatory by just looking at the code itself.
I think you have to consider things like reusability and unit-test-ability as well, and having all your code in a single function can make reasoning about it more difficult due to all the local variables in scope that you need to consider as possibly (maybe or maybe not) relevant to the block of code you’re reading.
That being said, when I look back on my younger, less experienced days, I often fell into the trap of over-refactoring perfectly fine linear code into something more modular, yet less maintainable due to all the jumping around. There is something to be said for leaving the code as you initially wrote it, because it is closer to how your mind was thinking at the time, and how a readers mind will also probably be interpreting the code as well. When you over-refactor, that can be lost.
So I guess in summary, this is one of those “programming is a craft” things, where experience helps you determine what is right in a situation.
Suddenly, you realize that the dozens of functions __need to be called in specific orders__, and they are each only ever used once. So really what you're doing is forcing someone to know the magic order these functions are composed in order for them to be of any use.
Oh my God.
Unfortunately organizing your code along the right lines of abstraction is something that just takes skill and can't easily be summarized in the form of "just always do this and your code will be better"
If you organize your code into units that are easy to recompose and remix, well you get huge benefits when you want recompose and remix things.
If you organize your code into units that can't be easily recomposed, then yes you've added complexity for no benefit. But why make units that can't be treated individually?
"As the complexity of the overall code grows, so would something that gets chopped into dozens of functions to the point of being unreadable."
So the answer to this is, "don't chop it into functions in a way that leaves it unreadable, instead chop it into functions in a way that leaves it more readable."
That may be unsatisfying, but it gets to the point that blindly applying rules is not always going to lead to better code. But it doesn't mean that an approach has no value.
Simply don't chop up your function until you need a slice of it somewhere else. Then refactor out the bit you need. You'll find out exactly which bits need to be replaced with variables and exactly where the slice needs to happen.
Same thing for copy pasta funcs -- the first copy is fine, the second one may be too, but after that consider extracting to a parameterized func (a permutation of the Go Proverb "A little copying is better than a little dependency.")
Really the question should always come up when there are more than say two ways to do things. If I can make a pizza from scratch, reheat a chilled pizza, create a pizza and chill it, reheat a half dozen pizzas, or make three pizzas of the same kind and chill them suddenly the useful abstractions are probably something you can figure out between those helper methods.
Honestly that is the real fear of the left way of thinking. If you add a quantity, whether to cook and whether to chill parameters you end up with a hard API where certain combinations of parameters don't make sense.
Have a clean API and make the implementation as simple as is feasible. Reuse via functions when it makes sense but don't add them willy nilly.
Aka "it is a craft and you figure things out" as someone said in the comments here
That's where nested functions show their true utility. You get short linear logic because everything is in functions, but the functions are all local scope so you get to modify local scope with them, and because the functions are all named, it is easy to determine what is going on.
What you're doing by breaking things into functions is trying to prevent it's eventual growth into a bug infested behemoth. In my experience, nearly every case where an area of a code base has become unmaintainable - it generally originates in a large, stateful piece of code that started in this fashion.
Every one who works in said area then usually has the option of either a) making it worse by adding another block to tweak it's behaviour, or b) start splitting it up and hope they don't break stuff.
I don't want to see the "how" every time I need to understand the "what". In fact, that is going to force me to parse extraneous detail, possibly for hundreds of lines, until I find the bit that actually needs to be changed.
Not every piece of code grows into a bug-infested behemoth. A lot of code doesn't grow for years. We're biased to think that every piece of code needs to "scale", but the reality is that most of it doesn't.
Instead of trying to fix issues in advance you should build a culture where issues are identified and fixed as they come up.
This piece of code will be a pain to maintain when the team gets bigger? So fix it when it actually gets bigger. Create space for engineers to talk about their pains and give them time to address those. Don't assume you know all their future pains and fix them in advance.
> In my experience, nearly every case where an area of a code base has become unmaintainable - it generally originates in a large, stateful piece of code that started in this fashion
In my experience it gets even worse with tons of prematurely-abstracted functions. Identifying and fixing large blocks of code that are hard to maintain is way easier that identifying and fixing premature abstractions. If you have to choose between the two (and you typically do), you should always choose large blocks of code.
The great thing about big blocks of code is that their flaws are so obvious. Which means they are easy to fix when the time comes. The skill every team desperately needs is identifying when the time comes, not writing code that scales from scratch (which is simply impossible).
One of the best reviewed functions I wrote at work is a 2000 line monster with 9 separate variable scopes (stages) written in a linear style. It had one purpose and one purpose only. It was supposed to convert from some individual html pages used in one corner of our app on one platform into a carousell that faked the native feel of another platform. We only needed that in one place and the whole process was incredibly specific to that platform and that corner of the app.
You could argue that every one of those 9 scopes could be a separate function, but then devs would be tempted to reuse them. Yet, each step had subtle assumptions about what happened before. The moment we would have spent effort to make them distinct functions we would have had to recheck our assumptions, generalize, verify that methods work on their own... For code that's barely ever needed elsewhere. We even had some code that was similar to some of the middle parta of the process... But just slightly didn't fit here. Changing that code caused other aspects of our software to fail.
The method was not any less debuggable, it still had end to end tests, none of the intermediate steps leaked state outside of the function. In fact 2 other devs contributed fixes over time. It worked really well. Not to mention that it was fast to write.
Linear code scales well and solves problems. You don't always want that but it sure as hell makes life easier in more contexts than you'd expect.
Note. Initial reactions to the 2000 line monster were not positive. But, spend 5 minutes with the function, and yeah... You couldn't really find practical flaws, just fears that didn't really manifest once you had a couple tests for it.
However, if you screw up linear code, you screw up locally. If you write poor small functions, the rest of the team screws up because they barely ever read the contents of your functions that call other functions that call other functions. I've had way more problems with stuff being called slightly out of order, than with large functions.
I think I’ve worked with some Python that looked and worked this way. I can’t place the details but probably in a processor pipeline running over a particularly hairy data format. Consider ancient specifications written by engineers talking on the phone encapsulated in relatively “modern” but still vintage specifications, sometimes involving screen-scraping a green screen mainframe terminal, wrapped in XML and sent over the internet. Anyway, point is I couldn’t agree more about stories.
It's like saying you need a reason not to add 4000 random jumps in your assembly code just to make it more difficult to read...
Source code tends to be very far removed from how computers execute anything, so I wouldn't use that as a justification for any sort of code style.
It is pronounced "documentation".
what we don't know is if it would have been possible to abstract those assumptions away so that functions could have been defined without them.
My longest code is always image processing. It's usually too hard to break up for the sake of breaking up. There's nothing to reuse between the calls to filters/whatever.
The fact that it works well and reads well (when it does, as in the parent's case), is.
Aside from those factors what else would be against it? Dogma?
To this end, I'd say it is important to be working in a language that avoids messing up the logic with boiler plate, or building some kind of mechanism (as dpkg did) to ease error handling and shove it out of the main flow; this is where the happy path shines: when it reads like a specification.
Isn’t that the fucking point? Having a 2000 line function is a code smell so bad, I don’t care how well the function works. It’s an automatic review fail in my book. Abstractions, closures, scope, and most importantly - docs to make sure others use your functions the way you intended them. Jesus.
Just don’t do that. Your code doesn’t have to have abstractions out the wazzo, but if your class (or method) is getting bigger than 1000 lines that’s a great sign that it’s doing too much and abstractions can be teased out. Your future self will thank you, as well as your team.
> You can't create the right abstraction until you fully understand the code, but the existence of the wrong abstraction may prevent you from ever doing so. This suggests that you should not reach for abstractions, but instead, you should resist them until they absolutely insist upon being created.
I am not saying you need to create complex abstract hiarchies right off the bat. But usually, it’s pretty easy to tease out a couple significant abstractions that are very obvious, and break down your classes by a factor of two or three. Just getting such low hanging fruit will prevent you from ever having a 2000 line long method.
And for the folks who are saying that they make sure to not add abstractions too early - are you disciplined enough to go back and add them later? I feel like if you’re the kind of engineer that busts out 2000 line methods, you’re also not going to refactor it as this method grows to 2500 or 3000 lines or beyond.
Probably most robust software you depend on is full of solid, quality abstractions. Learning to write code like this takes practice. The wrong abstraction might be wrong, but it’s one step closer on your journey to growing as an engineer. You won’t grow if you never try.
My point is the code smell isn't always accurate, and there are times and even for 2000 line monsters other devs agreed that it was the best way to hide complexity away from the rest of the codebase in that case. If we ever needed to factor things out (we never did), we could spend some effort and do it.
Some things are genuinely 2kloc-complex. Maybe not that many. Do check! But some are.
I'd like to clarify this was not necessarily 2kloc-complex, this was just 2kloc-long-and-not-really-meant-to-be-reused. It was a fairly long but linear process that was out of the ordinary for the rest of the codebase. It could easily have been split (hell, I had 9 fairly separate stages), but calling any of the intermediate stages out of order or without the context of the rest of the execution flow... would have been a foot gun for someone else. And, as time showed, we never needed those stages for anything else.
Why? Why can't the functions say "to be used by <this other function>, makes assumptions based on that function, do not use externally"? Breaking out code into a function so that the place it came from is easier to maintain... does not mandate that the code broken out needs to be "general purpose".
Good thinking. Now they’ll just add 50 flags and ten levels of nested ifs instead which is much simpler.
I read you have "end to end" tests.
One question though: Wouldn't each part benefit for having their own unit tests?
I found that I like to write tests at the level of abstraction I want to keep an implementation stable. I'd be totally fine if someone went in and changed the implementation details of that long process if needed. We cared that stuff got cleaned up at the end of the process, that the output matched certain criteria, that certain user interaction was triggered and so on... In that case it made more sense to test all our expectations for a larger scope of code, rather than "fix" the implementation details.
Tests usually "fix" expectations so they don't change from build to build. Tests don't ensure correctness, they ensure stuff doesn't alter unexpectedly.
Not necessarily better, especially since this allows for the case where individual unit tests pass fine, but the combined logic fails.
You are wrong here.
> this is one of those “programming is a craft” things, where experience helps you determine what is right in a situation.
You are right here.
The key insight on why giant linear functions are often more readable (and desirable) is because they allow you to keep more concepts/relationships simultaneously together as a single chunk without context switching which seems to aid our comprehension. An extreme proponent is Arthur Whitney (inventor of the K language) who writes very terse (almost incomprehensible to others) code so as to accommodate as much as possible in a single screen.
Two examples from my own experience;
1) I found reading/understanding/debugging a very large Windows message handler function (i.e. a WndProc with a giant switch statement containing all the business logic) far easier than the same application rewritten in Visual C++ where the message handlers were broken out into separate functions.
2) The sample code for a microcontroller showed an ADC usage example in two different ways; One with everything in the same file and another where the code was distributed across files eg. main.c/config.c/interrupts.c/timer.c/etc. Even though the LOC was <200 i found the second example hard to understand simply because of the context switch involved.
> Linear code is more readable
^ Wrong
> Linear code is sometimes more readable
^ Better
See for example Cyclomatic Complexity - https://en.wikipedia.org/wiki/Cyclomatic_complexity
I agree with this, but I think the combination of those two sentences winds up being "linear code is sometimes easier to comprehend, and sometimes not". The statement "linear code is easier to comprehend" is misleading. Your statement makes it seem like "at certain sizes" is the edge case; whereas, in my opinion, it's the only case that really matters. For a small enough block of code, "easier to comprehend" becomes a moot point.
> See for example Cyclomatic Complexity
I think that's only tangentially related. Cyclomatic Complexity deals with branching, which is somewhat orthogonal to refactoring out code to separate functions (though refactoring can make the branching easier to read, since it shows more in a smaller area).
But k has a small set of built-in commands and a built-in database; it was made for fast analysis of stock information, so with that you have everything you need and you use the same semantics. The only thing you need to know is the data structure and you can build whatever you need.
So in this way, it's very likely that, given two tables A + B and 'bunch of operations' X on A and 'bunch of operations Y' on B where Y depends on the result of X, and given the tasks to;
- create X' = X
- create XY' = X + Y
to implement XY without knowing X already exists rather than figure out X exists and reuse it.
The problem with not k (or programs written in similar style; it doesn't really matter what the programming language is), that we have learned to use the second style from the article, and, more extreme, to separate everything out in layers. You cannot even reach the data model without going through a layer (or more) of abstractions which makes it necessary not only to know the datamodel in detail but also find the matching findXinAandApplyWithYToB(). Where X & Y & A & B are often some kind of ambiguous and badly named entities. And then there is of course badly designed databases which is also quite the norm as far as we see, so there is a much lower data integrity which means that if you create something without checking all the code that touches it, that you might change something and the data becomes inconsistent.
I notice the same when working on systems built with stored procedures on MSSQL/Postgres; it is far quicker to oversee and (at least basically) understand the datamodel (even with 1000+ tables, which is rather normal for systems we work with) than it is to understand even a fraction of a, let's say Go, codebase. So when asked to do do a task XY', you are usually just not searching for X'; you are simply reading the data used in X & Y and whop up a procedure/query/whatever yourself. It's simply much faster as you have a restricted work surface; the model and sql (I know, you can use almost any language in postgres, but let's not here) and you can reason about them and the tasks at hand when you shut off internet and just use your sql workbench.
Yes, it is often just easier to write the linear code than figure out if you can reuse anything because the space is small. I think a good 'feeling' for this is, if you need internet search/package managers/copilot etc for something or if you can just write working code sitting on a desert island, quite possibly on paper. For instance, for C, asm (arm/68k/z80/8080 and older intel), k and some others I can write working code like that for non-toy applications in specific domains. And, at least for me, those languages lend themselves very well for this linear programming. Incidentally, but not related, this is for me also the most enjoyable way of programming; I kind of really hate using libraries. That's also because we work in heavy regulatory areas where you cannot just add them; you have to inspect them and sign of on them and, of course, most of them are terrible...
May we all be so lucky :-)
PS: You might want to consider adding your contact info. to your profile.
The problem with giant linear functions is that those concepts get separated by sometimes thousands of lines. Separating out the high-level concepts vs the nitty-gritty details, putting the latter in functions that then get called to implement the high-level concepts, does in my experience in most cases a better job of keeping related things together.
The issue is one of Policy vs. Mechanism - https://en.wikipedia.org/wiki/Separation_of_mechanism_and_po...
It is "Mechanism" which should be separated out and encapsulated while "Policy" (aka business logic) is what is better centralized as a linear (possibly large) function.
These pieces of code then often end up being private functions of a class. With state. Since they are private functions now, they are not really testable.
So now we got a lot of private functions that are only called once and typically modify side effect state. When these functions are grouped together with the caller, it is actually still a bit readable in simple cases.
But then after a while someone adds other functions in between the calling function and the factored out ones.
Now we have bits and pieces modifying different side effect state that no one knows if they are called from different places without getting a call graph or doing a search in the class file.
If you insist on making the code non-linear, I'd beg you to at least consider making these factored out private funcs inner funcs of the calling function if your language supports that. This makes it clear that these functions won't be called from anywhere else.
As with so many things in life, in a real codebase this is not an either/or, but an art of combining the two into something that stays readable and maintainable.
Will people bother testing all of them? Or will they write a single test, pass in a pizza and just glance at it actually working? My guess is the latter, as testing multiple branches from outside is often tedious, vs testing smaller specialized functions.
...that's basically why common sense and taste in programming is still required, it's not a purely mechanical task. That's also why I'm not entirely a fan of automatic code formatting tools, they don't understand the concept of nuance.
the cooking process won't though. stuffed crust? add some stuff in the middle. square? add some stuff in the middle. deep dish? add some stuff in the middle.
iterate a while and your "one golden test" is what falls down.
Refactor when those things are needed, right now the cooking process is stick it in a warm over for x minutes.
What are you testing there?
The oven was preheated? Put in an assert, that doesn't need a test.
That it stayed in for x minutes? You assuming the builtin sleep function is broken? Don't test library code, that's not your job.
That the oven actually preheated correctly, that was discussed in the article, the oven and it's preheat method should be a dependency that gets passed in, again not needed to be tested here.
Also in your example you are testing whether an if condition was evaluated as true.
Give me an example of a stuffed crust pizza cooking process that has a unit test which cannot be checked by looking at the resulting pizza.
But it's always implementation details all the way down!
If `prepare` is not worth testing, why would `createPizza` be worth testing? `createPizza` is someone else's implementation detail.
The basic tension here is between locality [0], on the one hand, and the desire to clearly show the high-level "table of contents" view on the other. Locality is more important for readable code. As the article notes, the TOC view can be made clear enough with section comments.
There is another, even more important, reason to prefer the linear code: It is much easier to navigate a codebase writ large when the "chunks" (functions / classes / whatever your language mandates) roughly correspond to business use-cases. Otherwise your search space gets too big, and you have to "reconstruct" the whole from the pieces yourself. The code's structure should do that for you.
If a bunch of "stuff" is all related to one thing (signup, or purchase, or whatever), let it be one thing in the code. It will be much easier to find and change things. Only break it down into sub-functions when re-use requires it. Don't do it solely for the sake of organization.
[0] https://htmx.org/essays/locality-of-behaviour/
What about for testing? What about for reducing state you need to keep in mind? What about releasing resources? What about understanding the impact of a change? Etc.
Consider an end of day process with 10 non-reusable steps that must run in order and each step is 100 lines. Each step uses similar data to the step before it so variables are similar but not the same. You would really choose a 1000 line single function?
For "use-case" code like this with many steps, you are typically testing how things wire together, and so will either be injecting mocks to unit test, in which case it is not a problem, or wanting to integration or e2e test, in which case it is also not a problem.
If complex, purely logical computation is part of the larger function, and you can pull that part out into a pure function which can be easily unit tested without mocks, that is indeed a valid factoring which I support, and an exception to the general rule.
> What about for reducing state you need to keep in mind?
Typically not a problem because if the function corresponds to a business use-case, you and everybody else is already thinking about it as "one thing".
> What about releasing resources?
Not a problem I have ever once run into with backend programming in garbage collected languages. Obviously if you are in a different situation, YMMV.
> Consider an end of day process with 30 non-reusable steps that must run in order and each step is 100 lines.
I would use my judgement and might break it down. Again, I have never encountered such a situation in many years of programming.
You seem to be trying to find the (ime) rare exceptions as if those disprove the general rule. But in practice the "explode your holistic function unnecessarily into 10 parts" is a much more common error than taking "don't break it down" too far.
Long, bespoke linear sequences can be hairy, but the tools to deal with them are present in current production languages without atomizing the code into tiny functions. Occasionally you can find a useful pattern that does call for a new function, and do a "harvest" on the code and get its size down. But you have to be patient with it before you have a good sense of where a new parameterized function gets the right effect, and where inlining and flagging an existing one will do better.
For me the biggest reason is state. The longer the function, the wider the scope of the local variables. Any code anywhere in the function can mutate any of the variables, and it's not immediately clear what the data flow is. More functions help scopes stay small, and data flow is more explicit.
A side benefit is that "more functions" helps keep indentation down.
At the same time, I don't like functions that are too small, otherwise it's hard to find out where any actual work gets done.
> Any code anywhere in the function can mutate any of the variables
Regardless of the language I'm using, I never mutate values. Counters in loops or some other hyper-local variables (for performance) might be the inconsequential exceptions to this rule.
> More functions help scopes stay small, and data flow is more explicit.
Just write your big function with local scope sections, if needed (another local exception to the rule above). Eg, in JS:
or even use IIFE to return the value and then you can use a const. "A function, you're cheating!" you might say, but my goal is not to avoid a particular language construct, but to maintain locality, and avoid unnecessary names and jumping around.> A side benefit is that "more functions" helps keep indentation down.
This is important and I maintain it.
See "Align the happy path to the left" (https://medium.com/@matryer/line-of-sight-in-code-186dd7cdea...)
It is also worth noting that solving this problem with function extraction can often be a merely aesthetic improvement. That is, you will still need to keep hold the surrounding context (if not the state) in your head when reading the function to understand the whole picture, and the extraction makes that harder.
Using early returns correctly, by contrast, can actually alleviate working memory issues, since you can dismiss everything above as "handling validation and errors". That is, even though technically, no matter what you do, you are spidering down the branches of control flow, and therefore in some very specific context, the code organization can affect how much attention you need to pay to that context.
> I don't like functions that are too small, otherwise it's hard to find out where any actual work gets done.
Precisely, just take this thinking to its logical conclusion. You can (mostly) have your cake and eat it too.
And after half a year of halving to always step into a method (or out of it) to continue reading or debugging, this resonates with me very much.
Discussion: https://news.ycombinator.com/item?id=12120752
[1] https://lihautan.com/json-parser-with-javascript/
For it to stay this readable, though, you'd need to move the functions around if you change the order of the steps. And that's fine if they're private functions, called only from the table of contents. Only, nothing forces you to keep them in order, or even to think about how it reads overall.
It often happens that functions start being reused in a way that can't be linearized anymore. Sometimes people give up and sort them alphabetically, or it's just random.
Another article that touches on this idea (among others) is https://loup-vaillant.fr/articles/source-of-readability which advocates that "code that is read together should be written together" (reading it made me confused until I realized it meant "placed together"), specifically "Consider inlining functions that are used only once".
https://en.wikipedia.org/wiki/Linear_B
Neither example is easily tested.
Neither support injecting the dependencies, which make mocking really difficult.
On the left, you're testing one big method with a whole bunch of conditionals, which leaves you with a whole ton of tests for that one big method.
On the right, there is a bake() method and it does oven.New(), but where does oven come from? Is it some global somewhere?
Pass in an order, assert the pizza that comes out is correct.
The entire function is a unit which can fit on my phone screen and has no external dependencies other than possibly oven, which was discussed in the article, it should probably have been passed in, aka dependency injection.
It isn't necessary with golang to do this at all, but it really helps build consistent structure throughout the entire app, so I do it.
Speaking from personal experience. I built a small golang process that ran on around 25k worker machines. It had to be bug free cause if it crashed and stopped running, it meant updating a whole lot of computers across multiple data centers, by hand.
We unit tested everything and the project worked out really well because of that.
Functions should exist at the level of concepts:
1. arr | flat | map | collect as HashMap makes sense.
2. CreateFlattenMappedHashMapFromArr does not.
boxPizza should take a CookedPizza, and BakePizza should take a RawPizza and return a CookedPizza etc.
Why would you care where the oven comes from, if the function deliver perfectly baked pizza? A unit test should test against the public interface and not be coupled to implementation details, since that will hamper refactorings.
> which make mocking really difficult.
Mocking is an antipattern anyway, and should be avoided except for nondeterministic components like current time or stateful external services.
> The code that is more easily unit testable, is the code I care about.
Author argues that his code is more readable. Sounds like you're saying that being unit-testable is more important than being readable.
> Neither example is easily tested.
Only if you're a unit testing zealot. Integration/E2E testing is easy for both.
> Neither support injecting the dependencies, which make mocking really difficult.
Mocking is not a virtue. Also, if mocking is the sole reason you're using DI, you're doing it wrong.
Come on, look in the mirror.
“I’m gonna return a pizza because I want a pizza”
When of course, what one really wants is a pizza in a box. And the oven objection is also kind of funny. It leads to a “but computers are so fast, why can’t they build me a new oven for each pizza?”
People think they want real-world analogies, which they hope will make code easier to reuse and maintain when what they really want are deep modules with clean interfaces, for which object orientation is not necessary in the least.
That's what Boxed<Pizza> is for, but this is more costly than a Pizza directly.
This is the same as someone arguing for scrolls when books with table of contents and appendices are far superior.
See my other comment here: https://news.ycombinator.com/item?id=37518275
>” From time to time, a complex algorithm will lead to a longer routine, and in those circumstances, the routine should be allowed to grow organically up to 100-200 lines. (A line is a noncomment, nonblank line of source code.) Decades of evidence say that routines of such length are no more error prone than shorter routines. Let issues such as depth of nesting, number of variables, and other complexity-related considerations dictate the length of the routine rather than imposing a length restriction per se.
If you want to write routines longer than about 200 lines, be careful. None of the studies that reported decreased cost, decreased error rates, or both with larger routines distinguished among sizes larger than 200 lines, and you’re bound to run into an upper limit of understandability as you pass 200 lines of code.”
[0] https://books.google.co.in/books?id=LpVCAwAAQBAJ&pg=PA174
Instead of repeating myself, i point you to my other comments in this thread for details.
Your only other option is to freeze them and never make changes, that doesn't happen much in real-world code (though it probably should).
Trying to in-place refactor those things is an exercise in frustration. That’s part of why they grow so large, from observing their proponents in action. They don’t actually know what the functions do, only where to add a new path and repeat themselves.
Huh, I can say the same about you by the reaction to this post.
99% of the time, you haven't actually come up with a good abstraction, so just write some linear code. Prefer copy/pasting to dubious function semantics.
If we consider the concepts orthogonally meaning we don't consider the fact that readability can influence scalability then "everyone" is fully correct. Linear code doesn't scale as well as modular code. The dichotomy is worth knowing and worth considering depending on the situation.
That being said I STILL disagree with you. Small functions do not cause readability issues if those functions are PURE. Meaning they don't touch state. That and you don't inject logic into your code, so explicitly minimize all dependency injection and passing functions to other functions.
Form a pipeline of pure functions passing only data to other functions then it all becomes readable and scalable. You'll much more rarely hit an issue where you have to rewrite your logic because of a design flaw. More often then not by composing pure functions your code becomes like legos. Every refactoring becomes more like re-configuring and recomposing existing primitives.
Small functions increase the chances of you having to do this. Larger ones decrease it, but can cause other issues.
Also, many small functions doesn't make code modular. Having well defined, focused interfaces (I don't mean in the OO sense) for people to use makes it modular. Small functions don't necessarily harm it, but if you're not really good at organizing things they definitely can obscure it.
And I totally agree, this is how you write large code bases without making them unmaintainable.
Where to go “linear” vs “modular” is an important design choice, but it’s secondary to the design choice of where to embed state-altering features in your program tree.
I think people dislike modular code because they want to have all the “side-effects” visible in one function. Perhaps they’ve only worked in code bases where people have made poor choices in that regard.
But if you can guarantee and document things like purity, idempotency, etc, you can blissfully ignore implementation details most of the time (i.e. until performance becomes an issue), which is definitionally what allows a codebase to scale.
12 months later you have:
> no_print = False
love this
Is print_table() + print_table_without_emoji() better than print_table(remove_emoji= False)?
Whenever function semantics need to change, everything degrades further because of refactoring uncertainties (=> you end up with even more parameters).
This will also be extremely resistant to optimization because even finding the "happy path" is non-trivial.
`print_table() + print_table_without_emoji()`
vs
`print_table(remove_emoji= False)`
vs
`switch table_name: case emoji: print(table) case no_emoji: print(table no emoji)`
Without understanding the implementation no one can truly say which is the better approach, but this idea of "surface area for bugs" is something that should be considered when approaching these types of decisions.
if needed remove the emoji, then print. if performance/table size is an issue, working via streams/generators/etc. should be on the (heh) table anyway.
But if you have conceded to being in quick&dir^H^Hpragmatic-land anyway, IMO both can be ok depending on the context.
You're coding for a future that might not exist. You might be coding for the wrong future and you painted yourself into a corner.
Been there, done that.
Maybe there's a way where I can code for every possible future with minimal effort. I'm talking about a pattern that isn't a form of premature optimization. Just a rule.
Your way of coding is, coding for the most probable future. Distinctly different of coding for every possible future.
> Ah, the good old premature abstraction.
The function will get called from different places. Once from its caller, and a second time from its unit test.
So you entire premise is wrong.
The amount of places I’ve worked at that don’t even have accessible DB schemas is mind-boggling.
The prepare and addtoppings functions should be one function, prepare effectively just fills in a struct and calls add toppings, its pointless to seperare them.
The Bake function simply prepares the over for cooking, which the author mentioned should be a dependency with a method and then factors 4 lines of code into a new function for no reason. The bake and bake pizza function should be one function.
You can then keep the box function as is.
That would be both easier to maintain and easier to read.
The box function is broken too. You box the pizza and then return the pizza...but the box is logically a wrapper for the pizza. `box(pizza)` should return a boxed pizza. A box with contents=[pizza]. Maybe some sauce and pepperoncini in there too.
Plus all these functions are impure. Which isn't always bad but if you can prevent things like boxing it before baking it, you should.
And what even... this entire example is just horrendous. You box the pizza and then slice the pizza? Ready = box.Close()? Can the Close() operation fail? And then the pizza is not ready? Why not throw an error, now the caller has to check if the pizza that got returned to them is even ready...? And that fact is even more hidden on the right side. Same for Sliced and Boxed.
I'd argue the entire box.Close() method is slideware and wouldn't exist since it likely is just a return true. You can just as easily just say pizza.Ready = true. Reading this code afterwards I would think there was some stupid requirement somewhere for a pizza.Ready property so someone added it and would check a commit log to see if it can just be removed.
Decent catch there though, the box can also be a dependency that get's passed in.
pizza.Toppings = getToppings(kind string)
That's a side effect.