13 ms·

My personal rule is: if a simple (<=10LOC) function is used fewer than 3 times in code, it's not really a function, it should be inlined. Only noobs avoid copy&

• scarface74 8y ago
  It’s a lot easier to read one function that calls 10 functions that are named what they do than to read one long 300 line function. Inlining functions is not a readability thing. It’s for performance when needed. In C, you can give a compiler hint to online a function and get the best of both worlds.
  • maxxxxx 8y ago
    "Inlining functions is not a readability thing." Sometimes it is for readability. For example we have code that serializes commands sent to a device. It used be built with a command pattern which was hard to debug. We have replaced it with a huge switch statement and now the code is much easier to understand and refactor. Obviously this approach often doesn't work but there are cases where inlining makes code easier to read.
    • scarface74 8y ago
      Even with using a switch statement, how would it be less readable to put each part of the statement in a function?
      • iainmerrick 8y ago
        Imagine there are two or three key variables, and a bunch of repeated blocks of code that modify some or all of the variables. (That sounds messy but it’s the kind of thing that could easily come up in an embedded controller.) If you turn all those code blocks into functions, it’s no longer easy to visually inspect the code to see all the places where variable X is modified. It might not be a net win for readibility. Personally, I wouldn’t define a function purely to contain a repeated block of code; I only do it if the code is related, i.e. all operating on the same data. In my first example, before trying to wrap things in functions, I’d try to figure out if some of those variables can be tied together into a single object with a nice clear interface.
        • maxxxxx 8y ago
          Good explanation. Thanks?
        • scarface74 8y ago
          If you turn all those code blocks into functions, it’s no longer easy to visually inspect the code to see all the places where variable X is modified. It might not be a net win for readibility. This goes back to not modifying global (or class global) variables and using a functional style. If you need to pass four or five variables put them in your languages equivalent of a pass by value struct. In the case of my favorite IDE, Extract Method and creating a struct/class from a parameter list is an automated, guaranteed safe refactor.
  • mtzet 8y ago
    > Inlining functions is not a readability thing. Inlining for readability is precisely what the linked article is about.
    • scarface74 8y ago
      If you are changing global state, and each function has an order dependency, I could see that. But it seems like even then his general takeaway is not to inline functions but not to change global state and use a more functional style of programming.
      • mtzet 8y ago
        I think the takeaway is both. Avoid state changes and use a functional style wherever possible. But, when you do need to make state changes, keep it inline rather than hiding it behind function calls. Better to have one long function, sectioned off by comments, than to have state changes with subtle order dependencies in several different functions.
        • scarface74 8y ago
          I would go one step further. Don’t section them off with just comments, use nested scoping blocks. At least most modern IDEs then can collapse the sections and you still have the concept of local variables.
        • lmm 8y ago
          Or use a state monad to distinguish between steps that mutate state and functions that don't, making your order dependencies visible and easy to work with...
          • scarface74 8y ago
            And this is how we end up with classes like AbstractSingletonProxyFactoryBean
            • 8y ago
  • xorcist 8y ago
    Hard disagree on this. I read quite a bit of code for a living and one of my single biggest complaints about most codebases are trivial functions (or useless objects). If your code does n operations in sequence, just list those operations. Don't hide them off somewhere. Code that reads linearly is by far the easiest to read. At least when you are familiar with the language and idioms. It's like the difference between a novel and a choose-your-adventure book. Modern IDEs help, but there's only so much they can do.
    • kazagistar 8y ago
      My favorite rule is orthogonal to length. Functions should ideally be at one level of abstraction. If you are doing 100 things at a single level of abstraction, and it flows logically, then by all means, keep it togeather. But if in a high level protocol specification you, say, concatenate strings and describe the details of fetching from files etc, then it might be better to isolate those. Of course, what exactly a level of abstraction depends on how you model, and personal opinion. But I find it a more useful starting point then "line count".
      • xorcist 8y ago
        Yes, absolutely! A hundred line function can be trivial, and a oneliner can be complex. It has nothing to do with function length but how much complexity and dependencies they isolate. What you say about specifications certainly rings true. In those cases the reader's preferred abstraction level is pretty much given.
    • jfries 8y ago
      Speaking of IDEs, a cool feature would be automatic read-only inlining for short functions. Could probably help understanding what's going on in code with lots of calls.
    • lmm 8y ago
      > If your code does n operations in sequence, just list those operations. Don't hide them off somewhere. In principle I agree. In practice, if your function needs to do more than say five things one after another then something has gone wrong. If you were sending a human a set of instructions on how to do something, that's the point where you'd start grouping them into logical "meta-steps" (e.g. prepare the ingredients, make the broth, prepare the meat, finish cooking the dish). Chopping a 12-step function into 3 4-step functions without regard for what each step actually does will make it less readable, just like doing the same to a recipe, or putting paragraph breaks and full stops into a passage at random. But a single function that does a long sequence of things is a code smell in the same way as an overly long sentence; it's well worth looking for a way to logically group these steps into a structure that will help the reader understand.
  • m0zg 8y ago
    If it calls 10 functions, then the equivalent "inlined" function written under my rule is at most 100 lines long, which means it fits on one page easily, and it's almost certainly easier to read than one where you have to follow 10 different function calls. Function naming usually sucks. As they say it's one of the hardest problems in computer science. If your code is 10 lines or less, it should be self-evident (perhaps with a short comment) as to what it does. Also, as an aside, inlining "for performance" with the inline keyword doesn't always inline. You have to use a GCC hint to force it.
    • icebraining 8y ago
      > If it calls 10 functions, then the equivalent "inlined" function written under my rule is at most 100 lines long Only if those 10 functions weren't already inlined themselves.
      • m0zg 8y ago
        But then they won't be 10 lines or less anymore.
        • icebraining 8y ago
          Yeah, but if you have A() calls B() calls C(), and they're all a few lines long, how do you decide which ones to inline? If you just do it as you're writing, don't you end up missing inlining opportunities further ahead just because you've already inlined before, and therefore the resulting function is too big to be further inlined?
          • m0zg 8y ago
            >> how do you decide which ones to inline Use whichever setup maximizes readability. Lift code out into functions if it's a logical, reasonably self contained block used 3 or more times. Use your best judgment, in other words. If you require me to jump around the file (or worse, files, plural) to read your code, make sure there's a good reason for all this cognitive overhead.
• fsloth 8y ago
  Obviously people are fairly opinionated about this. I agree with you 100% - code reuse is far too overvalued and copy and pasting has a bad name. If something "fits" a function then yes, by all means lift it as a function. But not all long snippets of code make sense when lifted out of the scope - there's no reason to wrap those. Most of all, forcing all code to fit "merely calling functions style" is an antipattern that can make the code less understandable.
  • m0zg 8y ago
    Good code is like a good book, it's written to be read. A lot of people don't get that. Small single-use functions are like footnotes.
    • quincunx 8y ago
      ... and maximizing entropy makes for a terrible read. Good code, like child poetry, should be allowed to rhyme.
    • pif 8y ago
      Fully agree with you. The problem is many people don't know how to read code properly: they try to read all the details at once, instead of surfing at the top level(s) and going deeper only when they actually need. Such people are the bad developers who write bad code bacause they cannot read. My personal mantra with copy&past while programming is: if your mouse gave you a little electric shock on pasting, would you paste it anyway?
• throwaway415415 8y ago
  I feel lucky that I learned about functions late in the game. I used to write php in long long blocks of code. When I learned about functions I was very reticent to use them and very seldomly did.
• huhtenberg 8y ago
  Unless, of course, these 10 lines are an invariant-preserving data manipulation code. If you inline that, then good luck ever needing to change that invariant.
• simias 8y ago
  No need to be adversarial, we're talking about coding styles. I actually sometimes write very short helper functions that are actually meant to improve readability by abstracting some details and making the code more self-documenting. For instance I have this code in my current application (a device driver): static unsigned get_machine_id(struct manager *mgr) { u32 conf = readl(mgr->regs + OFF_MACHINE_ID); return conf & 0xff; } It's called twice in the entire code. By you rule I ought to manually inline it but I think it'd actually make the code worse, not better. It's always a matter of balance, writing good code is an art. >Only noobs avoid copy&paste at all costs Only noobs rely on rules of thumb indiscriminately.
  • m0zg 8y ago
    Where did I say anything about relying on anything "indiscriminately"? Avoiding repetition at all cost is definitely a mistake. Avoiding repetition where it's justifiable could be a sensible choice. Though in your case I'd just say: u32 machine_id = readl(mgr->regs + OFF_MACHINE_ID) & 0xff; and be done with it.
    • Gibbon1 8y ago
      #define machine_id() (readl(mgr->regs + OFF_MACHINE_ID) & 0xff)
      • atq2119 8y ago
        Please no. C-style macros are too leaky.
    • simias 8y ago
      >Where did I say anything about relying on anything "indiscriminately"? Well maybe I read a little too much into your comment but I interpreted it as "if you have small functions called only a small number of times in your code then you're a noob". I attempted to provide a counter-example. I'm sure that you didn't mean it quite as literally but there probably are a bunch of young, inexperienced coders browsing this thread and I think these rules of thumb can be pretty counterproductive when they reach impressionable minds. Then you end up with people cargo-culting weird ad-hoc rules without even understanding the reasoning behind them. >Avoiding repetition at all cost is definitely a mistake. Avoiding repetition where it's justifiable could be a sensible choice. I completely agree on that.
• SuddsMcDuff 8y ago
  I think the mistake that people make with DRY is that they apply it at the textual level. They think that if they see the same particular sequence of characters in multiple places that they've violated the principle and need to refactor. I rather see it at the behavioural level. Important domain logic ought to have a single implementation to avoid discrepencies that can put the system in an inconsistent state. For example your e-commerce platform really ought to just have one way of calculating the total cost of an order. But, if you've copy pasted the boilerplate code that sets up your logging subsystem in a dozen different places, who cares?
  • C4stor 8y ago
    The ops team care. When they'll want you to switch to another logging output config, you'll answer one hour later saying "done". Two days later, you'll fix another two or three of those config. One week later, you'll fix the final one. It's annoying to everyone. If it's not config, it's library versions, it's a change in a "boilerplate" argument, ... Code duplication makes it hard to update all the code that should be "the same". Everyone involved in actually having code updated every now and then cares about it.
    • SuddsMcDuff 8y ago
      Rate of change of a particular piece of code is a factor too, given. In my experience logging boilerplate has a very low rate of change, so I figured it was a good example, but I guess everyones experience is different.
• kurtisc 8y ago
  >code is written once but read multiple times I'm familiar with the expression but it's not really written once, it's written and iterated upon. If I change something in a function that's called twice, I can be confident it will be the same change both places it's called: there's no programmatic way of verifying the change is the same if I copy and pasted, there I have to compare text. Humans aren't good at that. So in Carmack's example, what if someone adds side effects to the pure, inlined code which don't break anything at the time? They could cause issues when someone later assumes the inlined code is pure. Whereas, if you have pure function calls, this is all done for you by a machine designed for it.
• yason 8y ago
  I mostly follow the 0, 1, N rule myself. The second time the code is needed I start looking into making the snippet a function and there's no doubt of doing that on the third account. The reason I do it is having several small functions gives the program a custom language. So I can effectively start programming in my own semantic terms even if I was writing C. Building high-level constructs is easier in high-level languages, of course, but you'll always have to start with the lowest building blocks and even that helps regardless of the language! The small functions will be marked for inlining and practically also inlined anyway so there's no performance cost. But it's immensely more practical to hypothetically write: const size_t len = get_data_size(fragment); rather than: const size_t len = ((struct packet_header *)&fragment->databuf[4])->sz + fragment->offset; The latter one is easy too because you know the code but you will have forgotten a year later.
• Nursie 8y ago
  The counterpoint to this is that if you find a bug, it has to be fixed in multiple different places, and you have to remember where those are, and that they exist. I write java (or try to) in such a way that the major function calls logical, sensible smaller steps, and in such a way that it is readable. If there are, generally speaking, three things a method needs to do before it returns, I don't do two of them in separate methods and then dive into the minutiae of the third right there. So I'm afraid I don't really see it. Modularisation and encapsulation can help comprehension.
• soneca 8y ago
  That you took the time to edit your comment to say you don't care about downvotes is proof that you do care. It is ok to care, btw. You don't need to dismiss other people who don't agree with you though.
• chpmrc 8y ago
  Here's a counterargument to that: a developer wants to do exactly what's done in those 10 LOCs, but there's no function encapsulating that so the dev in question can't intuitively, or through the documentation, find a reusable piece of code that will save him/her 9 LOCs. So the dev goes ahead and writes the exact same logic one more time. And because it's almost impossible to find an identical piece of logic in a large codebase, you now you have 4 places where that logic is being used and will likely never be replaced with a single function call. Had you put that logic into a nice little function the first time, maybe with a bit of documentation, it would be reusable, better tested (because more people use it) and will save 9 LOCs every single time that logic will be used. But, hey, I'm a noob so what do I know.
  • ahaferburg 8y ago
    Just because you have some helper function somewhere in your codebase doesn't mean somebody else is going to find it. It's not uncommon that you end up with multiple versions of the same helper function. And even if you did reuse that code, you might end up with more work because while you can you use it in two places, the third place that also uses it now has slightly different requirements. Do you add a bool parameter to that function? Now you have two code paths in the function, is it still tested as well as you thought it would? Or you could inline it... You can't predict the future. It's not worth thinking along the lines of "but maybe somebody might use it later". Only if they really do is it time to act. Don't go looking for abstractions. Let them come to you.
    • chpmrc 8y ago
      > doesn't mean somebody else is going to find it You either failed to document it / name it properly or the other dev was too lazy to find it. Making 10 different variations of the same thing is purely a mistake, not a good reason to justify not encapsulating code into reusable chunks. > Now you have two code paths in the function But the whole point here is that you don't need to know how those code paths are implemented, only that they work the way they are supposed to. > You can't predict the future. Just because you can't predict the future doesn't mean you shouldn't prepare for it.
  • 8y ago
• C1sc0cat 8y ago
  Amen I wish I could up vote this 100 times - a modern compiler shouldn't need you to inline a function for speed.
• liopleurodon 8y ago
  couldn't you just "static inline ...." and let the compiler inline it? He is talking about C....
  • liopleurodon 8y ago
    ah, scarface74 said the same thing :)
• notacoward 8y ago
  > Only noobs avoid copy&paste at all costs. If only that were true. I've seen plenty of people with 10-20 years of experience do this too. You know those coding standards that absolutely forbid functions longer than X lines? They weren't written by noobs. They were written by old-timers who assume authority based on mere longevity. The fact is that there are tradeoffs. The "giant function" approach does create risk that code will be copied and tweaked incorrectly, or violate the assumptions under which the original was correct. This risk can be ameliorated with asserts and/or good tests, but it never goes away entirely. OTOH, the "splatter" approach can be disastrous for subsequent navigation and reading of the code, and the kicker is that it doesn't eliminate the copy-pasta problem at all. I've had to fix many bugs caused by copying between many small functions, followed by either not updating them consistently or someone calling the wrong one. Generally I don't like it, but that's also a bit language-specific. In languages that have explicit guard/context constructs or objects with RAII there's little excuse. In Plain Old C function nesting is a valid way to deal with the cascading-error-cleanup problem. It's still not the approach I prefer, but I'm not enough of a jerk to deride others as noobs for using it.
• Devagamster 8y ago
  I am not sure the downvotes are from people who disagree with you. I think more likely they are from people who take issue with the way you said it. Just food for thought...