9 ms·
I have had the pleasure to work with lots of other people's code of varying styles and quality. Nothing is harder to read and understand than code which is dee
by dzdt 10y ago
I have had the pleasure to work with lots of other people's code of varying styles and quality. Nothing is harder to read and understand than code which is deeply nested calls from one little helper (or wrapper) function to another. Nothing is easier to read and understand than code which just flows straight through from top to bottom of a big function.
There are other tradeoffs of code reuse and speed and worst-case-speed and probability of introducing bugs. If you haven't read the article, do, its worth it.
I love that Carmack tries to measure which styles introduce more bugs! Who else does that? Seriously, I would love to see more of that.
- andrewingram 10y agoI'd have thought at least a few people measure bug ratios of different coding styles? It's a big motivator in terms of deciding up best practices. And whenever I fix a bug, I try to identify the root cause. If it was a coding pattern to blame (as opposed to bad process or design) I look for ways to eliminate that pattern.
- hndl 10y agoWe do have Cyclomatic Complexity (https://en.wikipedia.org/wiki/Cyclomatic_complexity https://en.wikipedia.org/wiki/Cyclomatic_complexity). I see this often enough to understand how important it is.
- rakpol 10y agoIsn't cyclomatic complexity just about the number of paths through code? It seems like (statically resolved) nested calls, as long as they don't loop or branch themselves, shouldn't increase cyclomatic complexity.
- sb8244 10y agoThere may be subtle branches like error handling that can impact it for otherwise non branching code
- rakpol 10y agoI was thinking about the issue of nested code the other day -- isn't it (mostly) an editing environment problem? I mean, if my IDE can detect that a method is pure, couldn't it do a little magic (e.g. provide a different coloured background, some dotted boxes, etc.) and show me the inlined code right there? It doesn't seem wise to architect your codebase around the deficiencies in your tools, since one is (hopefully) going to long outlive the other. Related: [1]: https://dubroy.com/blog/method-length-are-short-methods-actually-worse/ https://dubroy.com/blog/method-length-are-short-methods-actu...
- tibbon 10y agoOh, i like this idea. Imagine your IDE just being able to substitute method calls that are just calling single line methods, for viewing only. Or even being able to refactor them in-line, but keep them in private methods?
- caf 10y agoParticularly if the editor can do things like constant folding and branch elimination based on constants that are being passed at that point.
- jakub_h 10y agoThat's just the editor doing the compiler's work at edit time, though. I know that Chuck Moore has always advocated extreme early binding, but that really is usually associated with Forth, not with mainstream languages.
- caf 10y agoI didn't mean that the editor would change what was presented to the compiler, just that it would simplify for the user when expanding the use of helper functions.
- slazaro 10y agoWith Visual Studio, you can visually inline the definition of a function below its call by selecting "Peek Definition", which is close to what you're saying. You can only do it one at a time, and it's for quick scanning of what the function does, though.
- StavrosK 10y agoI don't know, I'd prefer: config_filename = get_config_filename() config = read_config(config_filename) endpoint_url = config["endpoint_url"] auth_data = get_auth_data(endpoint_url) to just dumping all of that inline.
- deleted 10y ago[deleted]
- stonemetal 10y agoIs get_config_filename() a large function that gets the filename directly or is the end result nested another hundred lines of callstack down? If it is a large function that directly returns results then that is my pick, high level "story" with shallow, direct access to the details. No dicking about for half an hour trying to figure out where A, B, or C happen, yet hidden details for when you are focusing on a different part of the code.
- CyberDildonics 10y agoExcept that is a false dichotomy. This isn't about inlining 'pure' functions (I know your example is IO, but they aren't mutating your program's state), it is about avoiding functions that are only used once and/or that mutate state. I've used the same technique - instead of creating a function that might be awkward due to all the inputs and outputs, I'll create an inner scope {} with nothing around it. Only the variables that it modifies/initializes are left on the outside. This is a simple way to enforce more of a data flow style within a function and ends up being a useful organizational tool. It is also similar to 'let' in some languages.
- MyNameIsFred 10y agoThis. In the article, Carmack does suggest just this, although its not as explicitly called out. If your methods are functional, then int fancyNumber = calculateFancyNumber(); Can be inclined cleanly/equivalently as: int fancyNumber; { // [Mathy stuff redacted] fancyNumber = someIntermediateVariable / anotherIntermediateVariable; }
- imron 10y ago> Nothing is easier to read and understand than code which just flows straight through from top to bottom of a big function It really depends on how long the function is. Ignoring stuff like variable declarations and verifying constraints at the beginning of a function, I personally find anything longer than a screen or two is usually better off (in terms of readability) being in its own function.
- kminehart 10y agoGood Lord yes. When I was working on point of sale machines for Toshiba, there was this unbelievable depth of inheritance and ridiculously deep callstack constantly, and it made fixing any minor bug take weeks. * abstract class Connection would have maybe 3 or 4 methods; * DataConnection would extend Connection and add a couple more methods that were specific to some proprietary protocol. * POSDataConnection would extend DataConnection and wrap this proprietary protocol for POS machines * ControllerDataConnection would extend POSDataConnection because a Point of Sale controller is technically a POS machine with a bit more functionality (Really just a couple flags turned on). * There was plenty more in between; it's been so long now that I've forgotten it all. It's just like we all learned in college! Object Oriented programming is supposed to model real-life! Except no, it's not. That's stupid and complicated. Now the part of the OS that was C/C++ was absolutely beautiful. It took the UNIX style of programming / applications seriously; every little piece was its own program, and it worked flawlessly. Anyone could jump in and get to work immediately because it was so well written. You could follow any program top to bottom and it just... made sense!
- artursapek 10y ago> it made fixing any minor bug take weeks. My god. A lot of these fancy language features sound good on paper but can be so abused that even just coming to understand the code takes up too much space in your head. https://twitter.com/davidcrawshaw/status/507268175256231936 https://twitter.com/davidcrawshaw/status/507268175256231936
- donatj 10y agoThis is part of why I am so in love with Go. I have never read Go I couldn't understand. It's very clear at heart. I have read through much of the standard library and it all makes sense.
- MyNameIsFred 10y agoDo you think that's that's because the language inherently makes things readable, or is it simply because the language is too new to have popular anti-patterns?
- donatj 10y agoI had the "pleasure" of working on a system that made recursive calls that made other recursive calls that made other recursive calls, which made recursive calls to the database. I'd helped design it originally but this was not at all what I intended. I had a medical emergency though that knocked me out of work for quite a while and when I got back this unreasonable thing was what they had built.
- zump 10y agoYES. Nothing is more annoying than a pointlessly huge call tree where the function is called from a single place.
- critium 10y agoWhile I agree deeply nested calls are bad, I've also had the pleasure of inheriting a an application with several, muti-thousand line functions and those are not fun either. In code as in life, all in moderation.
- SomeCollegeBro 10y agoCouldn't agree with you more. I get the impression that a lot of people in this thread have never worked on behemoth, million-line enterprise code bases. Cognitively speaking, 4 nested thousand-line function calls is equally as bad as 15 nested 10-line function calls. > In code as in life, all in moderation. Beautifully said. Both methods (procedural vs. object oriented) give you different ways to shoot yourself in the foot. Instead of picking one or the other, it's more important to manage the scope at which your project grows. You have to avoid falling into the traps of each style.
- jblow 10y agoWait, you are comparing 4000 lines of code to 150 lines of code? How does that make sense? If you modify your example to 400 nested 10-line function calls, how does that change your comparison?
- jackmott 10y agoSometimes that is just inherent in the problem. If a a few thousand lines are actually needed to solve the problem, it will be somewhat hard to deal with whether they are all in one big function or broken up into many functions. One option may be marginally less annoying than the other, depending on what you are doing, or preference perhaps. But sometimes it is just a "Such is life" situation.
- MereInterest 10y agoI think it depends on whether the problem lends itself to abstraction or not. If I have a problem that requires 1000 lines to solve several large system of linear equations, I'm absolutely going to be separating that out. On the other hand, if I have 1000 lines of "if in country A, apply this tax rate", then I'm not going to separate it, because there is no underlying abstraction to use.
- Waterluvian 10y agoMaybe an editor can allow you to expand function calls so they appear in-line when you want to read them as such. Often I do have the nested call hell, but the alternative is an absurd amount of writing/testing things twice.
- jameshart 10y agoI don't think I can agree with "Nothing is easier to read and understand than code which just flows straight through from top to bottom of a big function" in the general case. I will agree that if you are coding in the kind of environment Carmack is talking about, where you are writing code which interacts with massive shared global state, that purely procedural structured code is easiest to understand, because you can be sure that you can see all interactions with global data structures in one place. But if you don't have massive shared global state, I'd argue that calling subfunctions is a hugely valuable aid to understanding, since it allows you to reason much more about the way data dependencies flow through code when you can be sure that a call to a function means that that function can only act on the data structures passed in to it and is guaranteed not to affect anything else. I would much rather in that case see: a = subOperationA(arg) b = subOperationB(a) c = subOperationC(a, b) d = subOperationD(b, c) return d than see those three suboperations inlined and have to figure out for myself how the data dependencies flow through the code.
- stcredzero 10y agoDecomposition into functions and routines can make some sort of conceptual sense. This can be like the "chunking" that advanced chess players do, allowing them to remember entire board positions. Often, programs that are a pleasure to read have a structure which makes internal sense or which can be attached to an outside concept. Doing this optimizes for programmer reading time.
- taeric 10y agoIsn't this a bit of a strawman, though? All too often, you instead get the likes of: a = subOperationA(a, b, c, d, e, f, g, h, i) b = subOperationB(a, d, e, f, g, h, i, j, k) c = subOperationC(d, e, f, g, h, i, j, k, l) return c And then some poor fool breaks each of those up into about 8 functions each. So, you click into subOperationA to find it is subOperationAa chaining to subOperationAb,.... My gripe is when we argue against having the full solution in your mind as you work the pieces. Logically, it makes sense to reduce everything down to named things. Cognitively, that is expensive. And yes, I'm responding with an equally opposing strawman. I am beginning to reject that there is a "one true answer."
- honkhonkpants 10y agoC++11 has given us a new and wonderful world of this small function debugging horror, since we can now gin up lambda functors that have no name, that have a type with no known name, and don't play well with the debugger (good luck figuring out why your program is spending all of its time in "operator()").
- AstralStorm 10y agoEven relatively old generally and lldb show location information correctly for lambdas. Templates are usually more of a pain to debug.
- honkhonkpants 10y agoHow do you mean they show them correctly? Perhaps you are right but I often see them attributed to std::function::operator() in the header <functional>, because someone has assigned the lambda to a class member field or function argument of type std::function. When the function has more than one caller it can be a nuisance trying to figure out which caller's lambda is the one being called. The only way I know of to identify them is to examine the full template expansion which is often a kilobyte of metaprogram gibberish.
- robotresearcher 10y agoCan you not see the call stack for your lambda, as for any function?
- DigitalJack 10y agoMy experience, limited though it is, is that there is a trade-off between how easy it is to write code vs how easy it is to read. Abstracting with functions is very beneficial to the writer because, well, they come up with the abstractions and know what they mean. I feel they are less helpful to the reader, except perhaps at a very cursory level. If the reader is trying to actually understand the code to be able to modify it, many abstractions are actually a hindrance. For example, in lisp, it's a fairly common practice to essentially write a DSL for the problem you are solving. It's a great tool to be able to do this easily and quickly. You can build massive and complex programs without overloading your brain. However, for a reader new to your code base, there is a huge cognitive load to try to decipher the DSL. It's intuitive for the writer, because they invented it, but for the reader, it is a hurdle to overcome. Once you learn someone's DSL, it's a very powerful tool now for you too. But when every project has it's own, it's really too much to bear.
- qwertyuiop924 10y agoIf you write and document your DSL well, the cognitive load is minimal, unless you need to plunge into the implementation. And it's not hard to write a DSL: I wrote what looks a heck of a lot like a DSL for a very simplistic piece of IF I wrote with maybe 5 exposed functions, and no macros.
- oselhn 10y agoFrom my experience a lot of nested functions usually means spagetti code created from huge functions which were split on radnom points to small functions (which mixed abstraction levels and still do multiple things at once). If you properly design your data model there will be no huge functions and no deeply nested calls.
- purplerabbit 10y agoYou can definitely break up a function's contents into logical nested functions... Why are you saying "random points"? Nested functions can actually allow you to even out your abstraction levels... Just group lower-level operations in more deeply nested functions. It's sort of the functional equivalent of extracting a group of similarly-leveled concepts into a new object in the OO approach. Not sure if you were talking about C++, where my point probably doesn't apply... (Lack of nested function support, AFAIK)
- chimprich 10y agoBig functions are hard to unit-test. The article includes comments from Carmack from 2014 saying he now favours breaking the code up into pure functions.
- wschroed 10y agoI wonder if we've over-focused on the inlining vs functions-for-modularity thing here. Fundamentally, the primary objective is to express code in a way that is clear to the reader for maintenance. Secondarily, the code must work with the compiler to achieve the desired level of efficiency in speed or resource usage. I have come to interpret this as: We are language designers. This isn't about functions. This is about building a language for the business case that is comprised of primitive expressions, means of combination, and means of abstraction. If the language is clear to the reader, expressing the problem well, it should be easier to detect problems or extend the existing language and its uses. We are language designers already: If you build a traditional class with a bunch of methods, that is a language with how to deal with the concept embodied by the class. It must be held to the same standards of any language, DSL, or API design. I like to remind people that we don't tend to dig into the code behind printf(); we trust what it does. We have years of experience using that primitive. It's a great example of a function that has been through many revisions due to security issues, untrustworthy in its inception. What is key here is trust that a primitive does as advertised so that one does not have to dig into its source code repeatedly. Nested functions are not an issue in the presence of trust. My "secondarily" clause has a fatal flaw: Many languages are not suited for building languages while simultaneously not trading off speed and resource usage. The ones with pre-runtime macros/templates assist us developers in the building of expressions beyond the limitations of the base language with minimal fuss.
- jakub_h 10y ago"This isn't about functions. This is about building a language for the business case that is comprised of primitive expressions, means of combination, and means of abstraction." Heh. Anyone up for a Sussman&Abelson drinking game? :D
- chocolatebunny 10y agoI think the biggest problem with the "big function" is that it tends to lead to code duplication. If taskY is in bigFunction, but taskY is needed in otherPlaces then people tend to just copy paste taskY in otherPlaces. If another developer joins the team, then he might not have noticed that there is a taskY and may end up writing his own, completely different, implementation in yet another place. For C, I tend to go the big function route but I usually try to put extra braces with comments to isolate subtasks from each other within the big function. Then I can pull out the tasks into functions if I see that I need them in more places. I don't know how my old code is doing so I don't know if anyone has followed my lead in those projects.
- jontas 10y agoI don't know if you read the whole article but the exact same thing is suggested towards the end.
- rimantas 10y agoProcedural code is easier to reason about. OOP is easier to change. Unless it is Evil OOP, with deeeeeep hierachies and done without understanding why and what for.
- userhacker 10y agoThere is a very interesting discussion on how to go about doing this in this video https://youtu.be/QM1iUe6IofM?t=37m41s https://youtu.be/QM1iUe6IofM?t=37m41s
- specialist 10y agoMy most recent real-life example: Jackson + Afterburner I hand wrote a LA(1) style parser (and lexer) for JSON, because I wanted to experiment with a source code generation notion I had (vs reflection, or bytecode generation). I was surprised that in micro benchmarks, Jackson + Afterburner is still twice as fast as my stuff. I had to understand why. Turns out it does the lex/parse equivalent of unrolling and inlining (cut and paste) absolutely everything. The code is utterly baffling. My implementation's jar is ~30Kb, fast enough (faster than all but Jackson + Afterburner, Boon), stupid simple, and works the way I want, so I'll keep using it for my projects.