22 ms·
Linear code is more readable
- patrulek 3y agoGood luck with finding proper line to make change or find a bug with single long linear function that is always a mess, because theres never time to refactor. I would rather not want to work with such code.
- kubanczyk 3y agoI can't believe that 55 years after Go To Statement Considered Harmful we are still operating at medieval level of "I recently found this piece of code and now I have opinions to share". If coding for an employer, it's their business. But for use in collaborative public projects, I want a linter with experimentally measured effect on readability. Not stories from the field.
- christophilus 3y agoI agree. I really hate having to jump all over a file (or multiple files!) for something that could fit into a single page of linear code.
- meitham 3y agoI 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!
- Jeff_Brown 3y agoSomeone 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.
- gabereiser 3y agoThere’s a whole style of coding dedicated to that very notion. It’s called test driven development.
- BigJ1211 3y agoI 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.
- corethree 3y agoIt's also a naming issue. A good name means I don't have to jump.
- readthenotes1 3y agoThose comments won't match the code in 6 months. 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
- Supermancho 3y agoDo 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.
- MathMonkeyMan 3y agoEach PR has 20 lines, and there are 30 such PRs over six months.
- Supermancho 3y agoApologies. 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.
- readthenotes1 3y ago600 lines is relatively short for some of the bad code that I have seen...
- deleted 3y ago[deleted]
- bsuvc 3y agoThe 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.
- whywhywouldyou 3y agoSo 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.
- dfee 3y agoDozens of functions need to be called in a specific order? Oh my God.
- professoretc 3y agoIn 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.
- raggi 3y agoIf you never have to write any tests, perhaps this is ok.
- yCombLinks 3y agoWhat 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.
- raggi 3y agoyou'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.
- yCombLinks 3y agoThose 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.
- raggi 3y agoevery single comment extrapolating a trival example is a strawman
- jpc0 3y agoYAGNI 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.
- realrains 3y agoMixing 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.
- jonahx 3y agoHard 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. [0] https://htmx.org/essays/locality-of-behaviour/ https://htmx.org/essays/locality-of-behaviour/
- ryanjshaw 3y ago> 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?
- jonahx 3y ago> What about for testing? 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.
- skinkestek 3y agoI am currently working in some "best practice" (according to its author) code with hardly an if statement. 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.
- Shoop 3y agoRelated email by John Carmack: http://number-none.com/blow/blog/programming/2014/09/26/carmack-on-inlined-code.html http://number-none.com/blow/blog/programming/2014/09/26/carm... Discussion: https://news.ycombinator.com/item?id=12120752 https://news.ycombinator.com/item?id=12120752
- DrDroop 3y agoIt 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. [1] https://lihautan.com/json-parser-with-javascript/ https://lihautan.com/json-parser-with-javascript/
- skybrian 3y agoThey 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.
- orblivion 3y agoAll of this "why your favorite best practice is wrong, actually" stuff gives me whiplash.
- nyanpasu64 3y agoI 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.html 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.com/view/news/169296/Indepth_Functional_programming_in_C.php 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 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".
- js8 3y agoI 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.
- deleted 3y ago[deleted]
- latchkey 3y agoThe code that is more easily unit testable, is the code I care about. 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?
- jpc0 3y agoHere is an idea for a unit test for this code. 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.
- latchkey 3y agoSince 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.
- deleted 3y ago[deleted]
- anon-3988 3y agounit 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. 2. CreateFlattenMappedHashMapFromArr does not.
- d-us-vb 3y agoThis 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.
- jraph 3y ago> When of course, what one really wants is a pizza in a box That's what Boxed<Pizza> is for, but this is more costly than a Pizza directly.
- sns989 3y agothis 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.
- Jtsummers 3y agoIt'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).
- atq2119 3y agoLiterally 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.
- Jtsummers 3y agoIt’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.
- gabereiser 3y agoI 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.
- rramadass 3y agoYour 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. See my other comment here: https://news.ycombinator.com/item?id=37518275 https://news.ycombinator.com/item?id=37518275
- gabereiser 3y agoDRY, 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.” [0] https://books.google.co.in/books?id=LpVCAwAAQBAJ&pg=PA174 https://books.google.co.in/books?id=LpVCAwAAQBAJ&pg=PA174
- rramadass 3y agoThese 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.
- t3rra 3y agohaving a poor taste is nothing to have confident with. The author sounds like he has never coded anything large and complex.
- wiseowise 3y ago> having a poor taste is nothing to have confident with. Huh, I can say the same about you by the reaction to this post.
- s17n 3y agoEveryone 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.
- corethree 3y agoWell 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.
- bcrosby95 3y agoI 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.
- jimbob45 3y agoHow 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.
- gorgoiler 3y agoEnd to end tests only, for you! They’ll find your bug in 30 minutes or you get your money back!
- meindnoch 3y agoI hate asking the question "is this function called from different places, or was it extracted only for aesthetic reasons?".
- corethree 3y agoThe function will/may get called from different places in the future. I am coding for the future.
- kaoD 3y agoAh, the good old premature abstraction. 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.
- corethree 3y agoBut what if I'm coding for the correct future? 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.
- mrkeen 3y ago>> The function will/may get called from different places in the future. I am coding for the 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.
- dllthomas 3y agoWould you hate it if it was really, really easy to answer?
- al05 3y agoFunctions have never been about simply being called different places. So you entire premise is wrong.
- jpc0 3y agoI 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.
- erhaetherth 3y ago> You can then keep the box function as is. 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.
- jpc0 3y agoThis 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.
- pechay 3y agoI don't like the side effects of the second addToppings. I'd much prefer pizza.Toppings = getToppings(kind string)
- userbinator 3y agoIt's more readable to the CPU too. Deeply nested code, especially with many functions that are called once, is really horrible to debug. Extract functions when you see obvious repetition, not just to appease some dogmatic abstraction goal. Incidentally, this also helps the CPU (cache locality). Along the same lines, I'd rather have a directory with several dozen source files than several dozen nested directories that may contain only one or two files each.
- realrains 3y agoI don't think the issue is about which code is more readable, but whether it's efficient for the CPU or the computer. Modern compilers optimize more than we think in production builds.
- deleted 3y ago[deleted]
- devjab 3y agoI actually sort of agree that linear code is more readable, but that’s not what makes good code practices alone. So while good linear code is more readable, at least in my opinion, it’s also a lot less maintainable and testable. I have a few decades of experience now, I even work a side gig as an external examiner for CS students, and the only real world good practices I’ve seen over the years is keeping functions small. I know, I know, I grade students on a lot of things I don’t believe in. I’m not particularly fond of abstraction, or even avoiding code-duplication at all costs and so on, but “as close to single purpose” functions as you can get, do that, and the future will thank you for it. Because what is going to happen when the code in those examples run in production over a decade is that each segment is going to change. If you’re lucky the comments will be updated as that happens, but they more than likely won’t. The unit test will also get more and more clunky as changes happen because it’s big and unwieldy, and maybe someone is going to forget to alter the part of it that wasn’t obviously tied to a change. The code will probably also become a lot less readable as time goes by, not by intend or even incompetence but mostly due to time pressure or other human things. So yes, it’s more readable, and in the perfect world you probably wouldn’t need to separate your concerns, but we live in a very imperfect world and the smaller and less responsibility you give your functions the easier it’ll be to deal with that imperfection as time goes on.
- mtreis86 3y agoI recently started reading Sussman's Software Design for Flexibility and what you write is directly in line with that book https://mitpress.mit.edu/9780262045490/ https://mitpress.mit.edu/9780262045490/
- xmcqdpt2 3y agoSure, it's less testable BUT in the specific case at hand it's all mutations that need to be performed in a specific sequence. IMO if you are taking an object through a specific set of states, you either split that and use types to mark the transitions (bakePizza takes a RawPizza and returns a BakedPizza, enforcing the order of calls at compile time) or you write one big function because it doesn't make sense to create a pizza and then not bake it before you box it. I obviously prefer the former for readability, correctness, and testability etc. However, in most PL changing the type of an object involves creating a new object and has a runtime cost. For hot code path, it makes sense to mutate in place, but in that case it's better to keep it all in one linear function.
- anoy8888 3y agoFor me , if a function is bigger than one page and I have to scroll , it means it is probably too long . If it’s length is less than one page , I don’t bother to break it down into smaller functions unless it makes sense
- nevertoolate 3y agoThe right side version has extracted functions from an arguably worrisome implementation on the left, then someone inlined it and left comments to explain the purpose of ?some lines? of upcoming code. Author wants to optimize readability, Carmack and others want to reduce complexity by eliminating local optima introduced by abstractions. Other people want to make a fashion style out of it. I’m thinking: how does oven work? Does it mutate the parameter, is it heating up at a constant pace? If oven mutates pizza why not the Box methods? Also if inline person likes inline why they don’t inline Box and Oven. Because they are called from some other places? Why not inline those as well? So many questions to ask. I’m not sure this is a clear win for either styles. Maybe we should ask chatgpt :)
- Mizoguchi 3y ago"The right side version has extracted functions from an arguably worrisome implementation on the left" Not amount of linearity and abstractions, or silly comments, can make bad code readable. I see this stuff at work all the time. To my team's defense I deal with a lot of chemists and physicists who like to write their own algorithms.
- jvans 3y agoIn my experience the more familiar that someone is with the code, the more they think pushing code into smaller functions is the correct path. They have already built up a mental model of the code at hand, so the cleanest implementation to them is one with very few lines. But the next person to come along has to bounce back and forth, performing mental stack push/pop operations to create the same mental model which is much harder to do when you don't have any of the original context
- sfn42 3y agoNot if the code makes sense. If the code is well written with elegant abstractions, slim interfaces and decent documentation, you often don't need to bounce around that much. For example how often do you read source code of your language's standard library? I almost never do, I mostly just look at method signatures and maybe read some docs if it's a bit complex or new. The whole point of interfaces is that you're not supposed to care how a method is implemented, only what it does which is explained by a combination of context, naming and documentation. But a lot of devs don't understand(or care about) this, so they write code that doesn't make sense and then it doesn't matter whether they made it linear or modular. They do things like make a service class where you have to call one method to get some data and then you have to call another to get some other data and then you have to call a third method to get some data that needs to be consolidated with the other two and now what the hell is the point of your service? It exposes all the internal complexity to the outside. You aren't supposed to force small methods, there's no point having 20 ~5-line functions that are all only called once and do super specific stuff and have to be called in the right order etc. That's not clean code, that's more like cargo cult programming. You are supposed to abstract things appropriately so that they make sense both to new and seasoned team members, are easy to reason about and hide complexity in places where the complexity makes sense. This is not easy to do but it is possible.
- Tomis02 3y ago> The whole point of interfaces is that you're not supposed to care how a method is implemented That's exactly how you end up with O(N^4) code. Your job is to care.
- garbanzoPDX 3y agoMuch prefer the right-side version. It's still "linear" at the top level and cognitive load is greatly reduced into small, single-responsibility, bite-sized functions. Plus—and sure, this might be a premature optimization—but future devs will thank you when it comes time to implement the "bake a calzone" feature.
- erfgh 3y agoPro tip: Use an editor that doesn't allow you to quickly jump to the definition of a function. You will make your code more readable because you will prefer to write linear code.
- Roark66 3y agoI too find the code on the left/red (linear) more readable. However the version with all the functions is quite extreme. When I'm splitting my code into functions I decide if something should be it's own function on the basis of: is this chunk of functionality required to be reusable? Am I repeating code, only slightly changed? If the answer is yes, a function gets created. I never do what I assume authors did here, find the smallest logical units code can split into and generate a bajillion functions. I'm not paid by the line of code after all. The same reason makes me like object programming (especially inheritance, abstract functions, operator overloading). IMO with a good IDE such code is much more succinct(within the constraints of the language) and more readable, but taking it to extreme is a mistake.
- osigurdson 3y agoThe one on the right is more readable, but takes things too far seemingly to prove a point. For example, "addToppings" clearly doesn't need to be a separate method.
- emodendroket 3y agoYeah, when it’s trivial code like this and doesn’t go on for ten pages that might be true.
- theptip 3y agoIt’s a matter of style, and like cooking, either too much or too little salt will ruin a dish. In this case I hope nobody is proposing a single 1000-line god function. Nor is a maximum of 5 lines per function going to read well. So where do we split things? This requires judgment, and yes, good taste. Also iteration. Just because the first place you tried to carve an abstraction didn’t work well, doesn’t mean you give up on abstractions; after refactoring a few times you’ll get an API that makes sense, hopefully with classes that match the business domain clearly. But at the same time, don’t be over-eager to abstract, or mortally offended by a few lines of duplication. Premature abstraction often ends up coupling code that should not have to evolve together. As a stylistic device, extracting a function which will only be called in one place to abstract away a unit of work can really clean up an algorithm; especially if you can hide boilerplate or prevent mixing of infra and domain concerns like business logic and DB connection handling. But again I’d recommend using this judiciously, and avoiding breaking up steps that should really be at the same level of abstraction.
- atoav 3y agoIf we go with the cooking analogy, if you have to describe to someone how to cook a meal, and at one part of the meal you have to put the fond in, it is reasonable to explain how to make the fond in a seperate section. The fond is it's own thing and it has one touching point with the food,therefore it is okay (or even benefitial) to move it out. Also: cooking recipes are also very abstracted. When they say you need to lightly fry onions they assume you know a way to cut onions and a lightly frying algorithm already. If they would inline everything it would become unreadable. Code is very similar. If you want it strictly without abstractions it will be as low level as your language allows you, and that is definitely not readable code. If you e.g. instead of using pythons "decode" method tries to do unicode decoding yourself it would become very hard to understand what your program is actually about. Now there are probably zero people who would do that, because the language provides a simple and well tested abstraction — but what makes that different from you creating your own simple and well tested abstraction and using that throughout the actual business logic of your code? The hard part is creating abstractions that are so well chosen that nobody will have to ever touch them again.
- okaleniuk 3y agoThis "level of abstraction" euphemism actually means "the level at which I'm not reading code anymore even and especially if I should". Of course, linear code is more readable! Linear everything is more readable. Have you ever seen a novel with "levels of abstraction" in it? But nobody reads the code anymore. Why bother? You're not going to stay on a single project for long enough for the attention investment to pay off. So the common best practice at the moment is to pretend that you read the code without actually reading it. For this purpose, the green code is much much better.
- japanuspus 3y agoI realize this is tongue in cheek, but really: Read code! If you ever start plateauing in your code skills, start digging into the code of your favorite open source project. Accept that things have been done in another way than you would for a reason and try to understand that reason. Try joining advent of code[0], and make sure to spend half you time block on reading and understanding alternative solutions. [0]: https://adventofcode.com/ https://adventofcode.com/
- mrkeen 3y ago> Linear everything is more readable. Have you ever seen a novel with "levels of abstraction" in it? Not if you're in the business of writing novels. What happens if you decide to edit out a scene - do you re-read the entire book to double-check that the deleted scene wasn't referenced anywhere?
- kristjank 3y agoI have been working on a system for programming some specialty hardware on customer premises for a while, and most of it was written in a pseudo-language implemented by another backend programmer. Think BASIC-like implements in a YAML file, with arbitrary python inserts here and there. Despite the code being not very visually attractive (long corridors of imperative statements reading and writing from SMBus addresses), I was always surprised how easy it was to maintain the code, and how quickly I could get back "in the zone" after not working on it for months. There is something painfully trivial about old clunky languages that makes them somewhat easier to get back into. The cost in abstraction capabilities is obvious though. The only reason I can afford to write concise, linear, imperative code for this project is its narrow, specialized scope that most of modern programs cannot afford to limit themselves to anymore.
- afandian 3y ago> Also, what happens if you pass a pizza to those functions twice? Are they idempotent or do you end up eating cinder? That is surely about state and mutable data, not code structure. And factored code makes it _easier_ to write more stateless code.
- mrkeen 3y agoYeah, that's the most frustrating part of the debate here. Arguing about the difference between: number := prepareZero addOne(number) addTwo(number) addThree(number) return number and number = 0 number += 1 number += 2 number += 3 return number When either of the following would be far better: addThree (addTwo (addOne prepareZero)) or return 0 + 1 + 2 + 3;
- dgunay 3y agoThe example code would be less distracting if it at least attempted to stick to the pizza metaphor in a meaningful way and weren't subpar Go code. `prepare` is a horrible name for a function. I would expect a seasoned Gopher to call it something like `NewPizzaFromOrder`. I don't see any reason for putting `addToppings` in its own function. If you have to have it, I personally would have made it a method on Pizza something like `func (p *Pizza) WithToppings(topping ...Topping) *Pizza { /* ... */ }`. Real pizza is mutable, so the method mutates the receiver. Why is a new oven instantiated every time you want to bake a pizza? You should start with an oven you already have, then do `oven.Preheat()`, and then call call `oven.Bake(pizza)`. You can take this further by having `oven.Preheat()` return a newtype of Oven which exposes `.Bake()` so that you can't accidentally bake something without preheating the oven first. Maybe elsewhere `Baker` is an interface, and you have a `ToasterOven` implementation that does not require you to preheat before baking because it's just not as important. Without changing the code, I'd also reorder the declarations to be more what you'd expect (so you don't have to jump up and down the page as you scan through functions that call each other). IDK I have to leave now but there are just so, so many ways in which the code is already a deeply horrible example to even start picking apart the "which is more readable" debate.
- siddharthgoel88 3y agoI noticed that people have already contributed great insights on readability and testability aspects which were my first thoughts as well on reading this blog. However, I do believe that there is no one right answer to this argument. And the right answer is with that team who in the end have to read, write and maintain that code. The metrics that I collect with my co-workers who work on same code base as me are * What is the cognitive load to grasp the code for members in the team? * How easy is it to onboard a new member to this team? * Are we able to move fast and have confidence in the code changes we make? In my opinion, metrics like these are usually the ones most of us care about in the end.
- weatherlight 3y agoso that team knows all future hire? I work on code bases that were written 15 years ago by a plethora of people in different imperative styles. where business requirements changed a lot over that 15 year period. There's a lot of spaghetti code. we found There's a strong correlation between method/function size and bugs. There's not a lot of confidence because there's implicit mutable state and side effects all over the place.
- js8 3y agoI think the fundamental problem is that, despite our wishes, there are programs which are inherently complex, and cannot be refactored into a simple, by-pieces testable, form. And if we try to do that anyway, all we end up with is just more fluff (mocking, I am mocking you) that hides the complexity. The internal complexity doesn't necessarily come from complex abstractions. Take for example some implementation of a tax code, i.e. code calculating taxes. There is probably gonna be a lot of interdependencies, dealing with special cases. That's your typical "business logic". This code is not inherently complex because the primitives are complex, but because there is a lot of dependencies in the calculation. That fact in itself makes it difficult to unit test. On the other end of the spectrum, we have something like a library of functions, for example, mathematical functions. The inner workings of how to calculate, say, a gamma function, can be very complex to understand, but the surface (API) of each of the function is very small, and that makes the library itself simple and easy to unit test. We can make an analogy with books instead of programs. On one end, you have a novel, which despite being written in a plain language, has many interdependencies of the characters interacting. You cannot "unit test" a novel by reading a single chapter, you have to read it all. You can have a summary of the novel (like the top function in exhibit B in the OP's example), but the summary of the novel is not exactly the novel, you're not really testing the novel if you read just the summary. On the other end, there are reference works like dictionary or encyclopedia. We can unit test these easily, since each entry should stand on its own (if you want to evaluate quality of a reference work, you can pick a few entries and test that, and it's gonna be pretty representative). They are not emergently complex like a novel is, despite the fact that entries might use specialized jargon and be harder to read.
- andrewjl 3y ago> That fact in itself makes it difficult to unit test. Verifying a tax code implementation is a good place to make use of property based testing.
- js8 3y agoI agree, and that's why I favor it to unit testing (although to be fair, they are pretty complementary, because each addresses different end of the spectrum). To properly unit test, you need to have a different implementation, which you can compare with, you cannot IMHO unit test under the same assumptions that the code makes.
- justanotherjoe 3y agoSure, you can put A, B, C, D side by side. But next time if you need to find D, you have to navigate A > B > C > D, with no other recourse. Often, you don't care about A, B, or C. Only D. And the benefit of A, B, C, and D being close together becomes immaterial. In real systems where things are spread, A, B, C, and D can be very far apart indeed. And it's totally fine! What matters is that from the starting position,let's say X, I can 'navigate' to A, B, C, or D, in an equal and speedy manner. Plus human brains love to navigate things in a 'spatial' way like this. It's natural. Really when you think about it, the perceived loss here is not that big compared to the benefits.
- exitb 3y agoThe straw man has been shot with silver bullets. Can we also linearise calls like box.PutIn(pizza)? What if it's a complex external API call that takes the pizza serialised to ProtoBuf and needs credentials that you'll retrieve from a configuration provider?
- dwb 3y agoI know it’s (usually, mostly) implied, but one of my dearest wishes for programming discourse is for people to say that something is more/less readable for them rather than declaring it a universal.
- agumonkey 3y agoI wonder if modularization is not a form of parametrization.
- zogrodea 3y agoI think it is, at least in some cases. I've found that Elm/React are like this with dynamic elements. First, you code the component you want with hardcoded styles and data. Then, you extract that to a function. Finally, you pull out the harcoded styles and data into parameters for that functoin. Now you have a reusable component.
- bogdan 3y agoThere is absolutely no doubt in my mind that that right variant is significantly better. I prefer it because of the smaller lexical scopes, because it's easier to test and most importantly to me it's easier to extend and to understand what the workflow intends to do. If I had to maintain code like this, I imagine in my day to day, I'll likely only have to extend it by only touching the `addToppings` function, the rest can stay the same. If I have someone new joining my team I can easily guide them to this `addToppings` function and ask them to add support for pineapple and ham, nobody needs to be overwhelmed by the entire system and their task would be done in no time. I do acknowledge the question is about readability but I don't think it's possible to ignore testability, manageability and extensibility. I think the inlined approach simply does not strike a good balance of the aforementioned.
- BoorishBears 3y agoAs someone who prefers the underlying of the non-linear right side example, it's written terribly code (which is puzzling since it was supposed to be the halo case) The prepare function is the main issue: it creates the pizza and adds toppings. If the pizza had been constructed at the top of createPizza, then `addToppings` `bake` and `box` were called, it'd be strictly clearer than it is now. Now obviously this is all from a contrived example, but I think the underlying lesson is: bad linear code is less tedious to deal with than bad non-linear code. With bad overly long linear code, at least the whole mess is in front of you. With bad non-linear code stuff is hiding stage-left, there's side effects that names are hiding, you're at the mercy of tooling for navigation, etc. Maybe if you know what you're making is doomed to be bad code (think convoluted business logic driven by the real world), maybe prefer linear?
- dzikimarian 3y agoWrong thing is discussed in this post. Code on the right isn't good because it's non-linear. It's good because it outlains business processe clearly, making it easy to get a grasp on it, if you never baked pizza before and aren't an author of the original piece. It's possible to write non-linear code using for eg unnecessary events or to much levels of abstraction and have the same issues for completely opposite reason.
- Mizoguchi 3y agoLet's heat up the oven then check if the pizza is baked.
- imtringued 3y agoPrepare, addToppings and bake are meaningless functions that serve no purpose. Meanwhile heatOven, bakePizza and box do have very good reasons to exist.
- urbandw311er 3y agoI’d like to also see the Rx example of this code. In my experience it would be vastly less readable but probably half the length.
- jraph 3y ago> I know this is a synthetic example but this kind of issue actually occurs in real code and sometimes causes performance issues. It is likely that this code should take the oven as a parameter. Providing it is the job of the caller. The caller might not care about the oven, does not know the processes needs a oven, or does not even have a oven. The injection pattern could be used.
- molly0 3y agoTestability matters more than readability - please separate different parts into different functions!
- agigao 3y agoPerhaps we can try to do it in a proper functional language? (ns restaurant.pizza (:require [restaurant.oven :as oven] [restaurant.package :as pack])) (defn make-order [size sauce cheese kind] {:size size :sauce sauce :cheese cheese :kind kind}) (def toppings-map {"Veg" "Veg toppings" "Meat" "Meat toppings"}) (defn prepare [order] (assoc order :toppings (:kind order))) (defn bake [prepared-order] (oven/bake prepared-order :pizza)) (defn box [baked-pizza] (pack/box baked-pizza :pizza)) (defn pizza [order] (-> order prepare bake box)) (comment (def order (make-order 26 "Tomato" "Mozzarella" "Meat")) (pizza order)) It's short and overwhelmingly granular, but for the sake of illustration. Large and complex codebases sliced up this way has not alternative in terms of ease of testing and reasoning about the code.
- al05 3y agoI still prefer the one the right. I'm able to skip entire sections of code, and assume what the function does. Only if I require details do I go deeper. The comments are metadata, and where function names are tied into the code. One is going to stay up to date. The other isn't.
- Ultimatt 3y agoMaybe more readable but less testable and less maintainable longer term.
- pif 3y agoThe problem with code styles is that most developers can only reason about information systems, where the only thing your code has to do is dispatch the right data to the right place. As soon as you try and write a function that actually uses the data, you find out that every book has been written for CRUD application programmers.
- al_be_back 3y ago>> this code makes no sense: why would you create a whole new oven to make a pizza? in real life... one can rationalize all sorts, but certain real-life metaphors don't have to map closely to the digital realm. if my CreatePizza function relies on remote/dynamic code/features (realtime functionality), then it's simpler and possibly safer to re-create than re-use. Depends on the use-case.
- Nevermark 3y agoI can imagine a new control statement with this type of syntax: code code uses (a, b, c, d) { // Step 5: Foo the bar code code } more code more code It's a block that defines the variables it uses, with no other access to the outer scope. It would help break up a linear function into blocks with clearer dependencies.
- tlarkworthy 3y agoJohn carmack said much the same and I have been following it ever since. Of course linear code is easier to read, if follows the order of execution. It minimizes eye saccades. Some code needs to be non-linear for reuse. Then execution is a graph. If you code does not exploit code reuse from a graph structure, do not bother introducing vertexes where a single edge suffices. http://number-none.com/blow/blog/programming/2014/09/26/carmack-on-inlined-code.html http://number-none.com/blow/blog/programming/2014/09/26/carm...
- Symmetry 3y agoSomething Carmack calls out but the OP doesn't is that if you can break out logic with no side effects into its own function that's usually a good idea. I think the left side would have benefited from pizza.Toppings = get_pizza_toppings(order.kind) in this case to keep the mutation of the pizza front and center in the main function here.
- olav 3y agoI wonder if there is some programming language that supports combining both styles: - A linear control flow - Named Blocks with explicit, named, typed parameters and return values I understand that one can use anonymous functions, immediately called to simulate this style.
- pushfoo 3y agoIf I understood you correctly, the ML family seems to come closest, especially Elm and F#'s use of |> syntax.
- faizshah 3y agoWhats not shown is the 10 other functions calling createPizza and bakePizza that can be tested by mocking that routine centrally. In the basic case, the linear version is better until the code is duplicated. Adding constants and function aliases before the code has duplicated is generally a bad idea.
- deafpolygon 3y agoI'm only a novice at programming, but my usual rule is if the indentation gets too far in, then it's time to put it in its own function. When it becomes hard to follow, I will put it in its own function doing The Thing(tm). But I won't break down every small logic into its own function - it's just too much.
- bcoughlan 3y agoThis was always my interpretation of "Flat is better than nested." from "The Zen of Python". I often run into conflict with developers who believe in the single return statement. This is flatter but irks a lot of devs: if (!condition) { return } more code return
- zelphirkalt 3y agoMutation everywhere, no thank you. This approach requires one to keep the state in mind while manually "evaluating" the mutations along the way, forming a picture, or whatever one uses, in mind about the created artifact.
- mrkeen 3y agoCorrect. Especially when there's mixing of in-place mutation and return-values. The first function in the green: func createPizza(order) { pizza = prepare(order) bake (pizza) box (pizza) return pizza } `prepare` transforms an order into a pizza, but `bake` and `box` mutates one in-place.
- mcv 3y agoI completely disagree with the article. The right hand side is far better. Not perfect; there are definitely a couple of things to improve, but it's better than just a big long meandering god function like on the left. It feels like the author is arguing to go back to the coding style of the 1980s. Big advantage of the right-hand style: the various steps are laid out in a simple 5-line function. You immediately see what making a pizza involves. Want to know more about it (like whether baking involves the creation of a completely new oven), you can zoom in on the details, but you never have to look at details that are irrelevant to you, unlike on the left side, where you have to dig through a page of code to figure out which part is relevant to you. Mind you, there are a lot of ways in which the right hand style could go wrong: if you don't separate your concerns, and have global or member variables manipulated by different functions in ways that are not immediately obvious, then superficially clean code could be hiding some terrible spaghetti. But at least the right-hand style punishes you for that and encourages you to do better (in fact, I'm currently refactoring a bit of code that did exactly that). The left hand side would allow terribly messy code with complex interactions between different parts of the code without making it obvious that those interactions are there, and will make it more intimidating to refactor them. Small functions are easier to test and easier to refactor.
- lucumo 3y agoI agree. The right hand side also makes it very easy to zoom in on the problem details. If the pizza is not properly boxed, I don't want to worry about whether the salami was sliced the right way. I can skip over that, and immediately zoom in on the boxing process. Pretending that a comment header is the same as a function is a bit silly. We can navigate to functions, not to comments.
- munro 3y agoI have to agree that the code on the left is far more readable (one function). I've worked with developers that have written code on the right (lots of functions), and it's always the worst to iterate on. The problem in the second approach is the functions aren't clean abstractions, they often hide logic&state transformations that only make sense in the calling context. So the dear reader is forced to jump back and forth between the multiple functions to understand the entire process. And just to throw a bit of shade, I encountered this type of programming more in webdev, and especially devops communities-- than with data scientists, ml, or data engineers. ;) And also when the director of eng wanted to get their feet wet every now and then.
- fjfaase 3y agoWhen you grow older, and become lazier, you only create function/methods when they need to be called more than once. Some languages, like C# and JavaScript, also allow you to define them local (inside a method). When these are used to perform some checks, I usually just place them before they are used, and when the perform some operation, I usually place them just below where they are called. The latter usually involves async of parallel execution. I just realize that this helps to keep the code more linear. So, I think I have a strong preference for linear code.
- FrustratedMonky 3y agoSeems like the key takeaway was adding comments.
- nevir 3y agoIt really all boils down to cognitive load: Can the average dev keep all of the variable states & side effects of the function in your head as they read through it? Great! Linear may be a good fit. Or does one need to jump up and down in the function to /really/ understand it? Probably time to consider abstracting it.
- 000ooo000 3y agoPretty lame contrived example; easy to make a case for inlining when your functions are 5 lines long. In any case, think I'll go on ignoring dogmatic coding advice.
- BenFrantzDale 3y agoThis blogger obviously wouldn’t get along with Sean Parent of Adobe. It’s old but I have everyone on my team watch this “no raw loops” presentation: https://youtu.be/W2tWOdzgXHA?si=4LKv1-sau60U63op https://youtu.be/W2tWOdzgXHA?si=4LKv1-sau60U63op in which he identifies reusable patterns hiding in code (“That’s a rotate!”) I myself was skeptical at first but have found over the years that breaking functions into pieces is the only way to maintain short functions that can be reasoned about in isolation, and as a side effect, surfaces reusable code. If you can’t write functions that easily fit on a page, I posit you don’t actually know what the function is supposed to be doing, and there’s probably a bug. (If you can’t hold the whole function in your head, how can you be sure there isn’t a bug?)
- mcemilg 3y agoBeside this, I also hate navigating through modules. I would like to see a part of code in a single file if it will not used anywhere else or the thing is very abstract.
- delbronski 3y agoPersonally, I find linear code more readable when I have context. The pizza example reads better linearly because is easy to figure out the context. But when I have no context (I enter a new code base) linear code is harder for me to reason about because at that point I’m just trying to understand how all the pieces fit together. At this point having everything stuffed in one place makes figuring out the higher level picture pretty difficult. I recently had to update a 1000+ lines of code function with very specific business rules that I had no context of. I’m sure for the developer writing it at that time it was easier to put everything in one big function, but it was pretty hard to figure out everything that was going on in there. I had to refactor it into a few smaller functions in order for me and the team to figure out what was actually going on and how we could fit the new business requirements into it.
- maxZZzzz 3y agoTwo points * functional style makes it easier to split stuff since you mostly transform immutable data In the Article IO, State (side-effects) and Data transformation is mixed in both versions. That leads to unnecessary complexity. In that case worse if it hides in sub-functions. But separate it and the right version is better. (you can answer the question about idempotency easily now) * Comments over blocks of code are harder to keep in sync with the code below, they require more discipline from all team members So in theory they are nice, but you will never have them unless you enforce them through proper code reviews.
- Micoloth 3y agoI agree with the Functional style part, but hardly disagree with your second point. Because of the fact that, keeping comments over blocks of code in sync with the code below requires the same exact amount of effort (arguably less) as keeping function names (and ideally, documentation) in sync with the code inside.. Except that, if the second one is not done, that is a lot more dangerous than the first- exactly because, every time you define a function, you are declaring an abstraction, and if the abstraction changes silently, that's where the real mess begins. That's what ends up happening in my experience. (See also sibling other comments about the proliferation of flags in a function signature) (Needless to say, I strongly resonated with the OP as I also love linear code with comments, but in the end it's also a matter of taste..)
- DanielHB 3y agoyou can split large linear function into smaller ones with this one simple trick: function myFunction() { /************************************ * Subsection 1 ************************************/ // code /************************************ * Subsection 2 ************************************/ // code /************************************ * Subsection 3 ************************************/ // code } better than splitting into multiple function calls with a bunch of variable-to-parameter and return-type to variable renaming going on. Helps if your language allows you to limit some variable scopes, but usually I wouldn't bother
- dvvolynkin 3y agoVery much depends on the naming and arch. If the naming and architecture are good, then it reads like a book.
- syncurrent 3y agoDrakon uses Silhouettes to show code linearly and abstracted at the same time: https://drakon.tech/read/silhouette https://drakon.tech/read/silhouette
- samsquire 3y agoThanks for the post. I do not enjoy navigating and bouncing through 100s of files to work out how something works. Where algorithms are obfuscated and there's indirection everywhere. I enjoy reading dense code where everything is clearly linear because I do not need to context switch. But when you need to change something, you probably prefer the many function approach.
- bluGill 3y agoI couldn't read the examples (on mobile), but in general I like more functions because it it is easy to skip over details I don't care about. If I'm interested in the oven preheat (his example) i'll dig into it, but if not I'll skip over that part to the toppings function.
- zoomablemind 3y agoI don't think the author's point is truly contrarian. The author is a proponent of clarity in the code, and of the readability. The deal here is that both versions are fairly readable, written by someone with intent to make it clear what the code should be doing. As a result the two versions are just examples of two expression styles, while the focus is on showing how the transition between these styles could be done. What's worth underscoring here is the cohesiveness of stretches of code, such that their execution could be summarized by a descriptive function name. Often in grand god-functions the contexts are so intertwined and mixed that it is hard to see cohesiveness in stretches of code. Thus, the refactoring is very much a tool to creating such cohesiveness and proper logical sequencing. Scooping out the code into separate function or commenting it out is more of a style judgement. Though putting the code into a function with a descriptive name indeed enforces this sort of analysis.
- 0x445442 3y agoThe differences are magnified when arbitrary test coverage metrics are imposed. The first example is easier to write tests for because all the conditionals are easier to scan. The second, green, example requires the test writer to follow multiple stack frames to write the test unless there’s some mocking and spying framework at hand.
- timwaagh 3y agoyou call your blog separateconcerns.com and then advocate for the opposite. functions have their purpose. one of them is separating concerns as is done here. the code on the right is probably better than 99.9% of code out there. there is even an academic book (normalized systems theory) which claims that having more concerns in one function, will inevitably cause an explosion of your codebase where you have to write a ton of code for a very small change. i doubt the validity of the proof they provide for this, but its something to keep in mind as i have not seen anyone more serious than I claim that it is wrong.
- regularfry 3y agoThe core lesson here is that "readability" is personal, and any attempt to reason about "more" or "less" that doesn't translate that into more specific, measurable outcomes (like time to find a bug, or time to add a new feature) is a very good way to nerd-snipe a large number of people into creating a lot of hot air.
- alexbezhan 3y agoI write top-to-bottom code almost always. And prefer not to have separate functions if I don't need them. Here is the real example I'm building CRM https://www.youtube.com/watch?v=l4QjeBEkNLc https://www.youtube.com/watch?v=l4QjeBEkNLc
- chpatrick 3y agoWhile I do like linear code the bigger problem is that everything is in scope. If you break it apart into separate functions you can clearly see the inputs and outputs.
- lost_tourist 3y agoRight, you can read both but it also requires more brain power to separate out when not in separate functions. It's also easier to unit test as the code grows larger.
- sunwukung 3y agothis - this is the reason. Large functions accrue state and state begets bugs over time.
- jb3689 3y agoLeft case has too much scope. I don’t know at a quick glance if a variable from line 1 is used on line 200
- pierrebai 3y agoThe linear version is hard to test. The split-function one is much more testable. There is also that thing called complexity, which increase with function length and has been proved to correlate with bugs count. The problem with the example is that it is both extremely artificial and shows a single use case. Even with the artificiality, one can easily imagine baking a calzone instead, which could reuse all the factored-out oven functions in the split version. (The comment about pizza vs baked pizza is one about using typing to encode your logic, but is separate from the issue that your functions should do one thing.)
- vaughan 3y agoThe real issue is plain text, and files and folders. File names, folder names/hierarchies, function names, class names are all _arbitrary_. You could randomize them all and your code would still run. What is not arbitrary is: the call graph, and the data flow/dependecy graph. Every line/block of code could be wrapped in a function. And classes...your class methods are just functions with an implicit parameter of an object of a certain type...and practically, not the entire object, just the parts it that it actually uses in the function body. So if you just focus on what your functions do, the boundaries and groupings of your code will become self-evident.
- kuchenbecker 3y agoTo add to this: the code you are writing and how you access it can be distinct. A static function with all parameters available is accessible but hard to use; adding a builder layer in-front makes it usable without changing the logic. CLI, RPC, Rest, are all different methods of interfacing with the underlying core function. I too-often see folks make a "microservice rpc server" rather than "function exposed via rpc microservice".
- dclowd9901 3y ago_sigh_. OP got really hung up on the example, and it ruined the article. There are certainly cases where linear makes sense. But beyond the length of your ticker tape memory is about it, and since that varies quite a lot from person to person, I like it best if I can choose the level of abstraction with which to read code. This has never been a problem. Where indirection becomes a problem is inheritance and black box operations (as you might find in rails-y) frameworks. Django’s block model of extension for templates is so devious it should be considered felonious.
- k3vinw 3y agoDesign patterns that are used too soon can contribute to less readable code too. Like implementing the strategy pattern when a simple if else would be much more succinct.
- namelosw 3y agoHeck no. If you do some "real world pizza making" instead of toying, that function would be like at least 1k lines, including how you carefully shape the dough, how to handle exceptions when you tear some holes, and how you should observe and rotate in the oven by how much, how you should redo it if the roller blade just didn't cut through properly, so on and so forth. Of course it's better to have top-down overview like prepare -> bake -> box otherwise the readers will surely lose themselves in details without figuring out what is happening. People in the game industry told me their horror story of helping designers with a Lua script that they were writing over the years. And it turned out the "Lua script" was a single file, with 100k+ lines, that bearly had several functions in it. That would be SO linear.
- layer8 3y agoLinear code can increase the state that you need to hold in your head while reading through it. You typically can’t just start reading in the middle and understand what is going on, because you have to trace the evolution of the state up to that point. Breaking the code up into smaller functions can reduce what you need to keep in your head, if the functions can be understood standalone just by their parameters, and if any side effects they may have on their parameters (in case of mutable objects) are straightforward enough to understand from their naming and/or comments (rather than from their implementation). One purpose of functions is to separate interface from implementation. If for some part of the code an interface is easier to understand than the implementation, then that’s a clear case for making it a separate function. The points of separation should therefore be the points where the least context is needed to understand the seperated-out operation.
- weatherlight 3y agoThe code in the red is imperative, with implicit state all over the place. I would argue that it's not linear, you have to keep all those mutable values and state transitions in your head as you read what's happening from top to bottom. It's not extensible, It's not composable, It's hard to test, and frankly, it's complicated and complex for no other reason than a particular type of engineer thinks its easier to read, because they feel all programming should be imperative. Don't get me wrong, there's a time and a place for this style of programming. (Manual memory management, algorithmic design that maximizes speed or memory usage, or even taking a bunch of services and dictating the order in which they are supposed to be executed.) For business logic, especially the type thats supposed to model "the world," this is terrible code. How is the bottom, not better code? type Oven struct { Temp int } type Box struct { // Box properties } type Pizza struct { Base string Sauce string Cheese string Toppings []string Baked bool Boxed bool Sliced bool Ready bool } type Order struct { Size string Sauce string Kind string } func preparePizza(order *Order) *Pizza { toppings := map[string][]string{ "Veg": []string{"Tomato", "Bell Pepper"}, "Meat": []string{"Pepperoni", "Sausage"}, } return &Pizza{ Base: order.Size, Sauce: order.Sauce, Cheese: "Mozzarella", Toppings: toppings[order.Kind], } } func bakePizza(oven *Oven, pizza *Pizza, cookingTemp int, checkOvenInterval int) *Pizza { // Simulate oven heating for oven.Temp < cookingTemp { time.Sleep(time.Duration(checkOvenInterval) * time.Millisecond) oven.Temp += 10 // Simulate oven heating } pizza.Baked = true return pizza } func boxPizza(pizza *Pizza, order *Order) *Pizza { box := &Box{} pizza.Boxed = true // Simulate putting pizza in box pizza.Sliced = true // Simulate slicing pizza pizza.Ready = true // Simulate closing box return pizza } // I just need to really understand this imperative part // this is the meat and potatoes func createPizza(order *Order, oven *Oven, cookingTemp int, checkOvenInterval int) *Pizza { pizza := preparePizza(order) pizza = bakePizza(oven, pizza, cookingTemp, checkOvenInterval) pizza = boxPizza(pizza, order) return pizza }
- weatherlight 3y ago
- memorythought 3y agoThis paper[1] suggests that there is a 50/50 split amongst programmers in the way in which they trace programs: > Given a straight-line program, we find half of our participants traced a program from the top-down line-by-line (linearly), and the other half start at the bottom and trace upward based on data dependencies (on-demand) So it's possible that both viewpoints are correct in some sense and we should pursue languages which allow us to switch between the two viewpoints. [1] https://arxiv.org/abs/2101.06305 https://arxiv.org/abs/2101.06305
- vaughan 3y agoInteresting read. It’s amazing more people don’t use runtime variable value annotation tools like Wallaby.js, or a debugger. So much time spent mentally remembering what is in what variable based on the naming. I often find myself adding “// e.g. foo, bar” to show example cases for some lines of code…like recedes for example. Wallaby.js is a godsend for this though.
- frodowtf 3y agoOr you use the correct types and avoid these "problems" alltogether. It's not possible to bake in a cold oven. Why does your type allow it then? Why don't you encode the state directly? oven := ColdOven.heat() bakedPizza := oven.bake(pizza)
- wduquette 3y agoFor many, many years, I've adopted the OP's choice of style, using what I call "FIRST/NEXT" comments to divide the function into paragraphs: // FIRST, Create the pizza object ... // NEXT, Add the toppings ... // NEXT, Heat the oven ... By all means, move a "paragraph" into its own function if it's called more than once; but otherwise this provides a number of useful features: * The FIRST/NEXT comments serve as useful headers, making it possible to navigate the function without reading the code in detail. * I know that no one's going to call one of the blocks from outside. * I can see at a glance what chunks of code go together. I've often gone back and read code I wrote five, ten, twenty, thirty years ago using this method, and found it perfectly readable.