7 ms·

> The complexity (sum total of possible interactions) grows as the number of lines within a functional boundary grows. That's only 1 part of the complexity equ

• jjnoakes 5y ago
  I feel like this is only a problem if the small functions share a lot of global state. If each one acts upon its arguments and returns values without side effects, ordering is much less of an issue IMO.
  • ajuc 5y ago
    Well, if they were one function before they probably share some state. Clean code recommends turning that function into a class and promoting the shared state from local variables into fields. After such a "refactoring" you get a nice puzzle trying to understand what exactly happens.
    • virtue3 5y ago
      I've seen threads on this before but the "goto" (couldn' t stop myself) reaching of object oriented-ness to "solve" everything is really frustrating. I've found the single greatest contributor to more readable and maintainable code is to limit state as much as possible. Which was really hard for me to learn because it can be somewhat less efficient, and my game programmer upbringing hates it.
      • JamesBarney 5y ago
        Sometimes eliminating state can also mean increasing complexity and lines of code tremendously.
        • TeMPOraL 5y ago
          A lot depends on what your language and its ecosystem can support. For instance, the kind of monadic stuff people do with Haskell and Scala can compress programs tremendously, but then I've worked in a codebase that tried the same things in C++ - and there, the line count expands, because the language just can't express some of the necessary concepts in a concise way.
    • josephg 5y ago
      In javascript I sometimes break up the behaviour of a large function by putting small internal functions inside it. Those internal functions often have side effects, mutating the state of the outer function which contains them. I find this approach a decent balance between having lots of small functions and having one big function. The result is self contained (like a function). It has the API of a function, and it can be read top to bottom. But you still get many of the readability benefits of small functions - like each of the internal methods can be named, and they’re simple and each one captures a specific thought / action.
      • BigJono 5y ago
        If you're calling those functions once each in a particular order then I can't possibly figure out what that does for you that whitespace and a few comments wouldn't. How does turning 100 lines of code into 120 and shuffling it out of execution order possibly make it easier to read?
        • nojokes 5y ago
          A recommended reading https://www.amazon.com/Refactoring-Improving-Existing-Addison-Wesley-Signature/dp/0134757599 https://www.amazon.com/Refactoring-Improving-Existing-Addiso...
        • throwawaythekey 5y ago
          I coded this way for a while and found it makes the code easier to read and easier to reason about. Instead of your function being func foo() { // do A.1 // do A.2 // do B.1 // do B.2 // etc... } It becomes func foo() { // do A // do B // etc... // func A()... // func B()... } When the func is doing something fairly complicated the savings can really add up. It also makes expressing some concurrency patterns easier (parallel, series etc...), I used to do this a lot back in the async.js days. The main downside seems to be less elegant automated testing from all the internal state.
        • josephg 5y ago
          No; I wouldn't do it if I was just calling them once each in a particular order. And I don't often use this trick for simple functions. But sometimes long functions have repeated behaviour. For example, in this case I needed to do two recursive tree walks inside this function, so each walk was expressed as an inner function which recursively called itself, and each is called once from the top level method: https://github.com/ottypes/json1/blob/05ef789cc697888802e7867f0e06d64b2b0471ba/lib/json1.ts#L422-L579 https://github.com/ottypes/json1/blob/05ef789cc697888802e786... I don't do it like this often though. The code in this file is easily the most complex code I've written in nearly 3 decades of programming. Here my ability to read and edit the code is easily the most important factor. I think this form makes the core algorithm more clear than any other way I could factor this code. I considered smearing this internal logic out over several top level methods, but I doubt that would be any easier to read.
    • chii 5y ago
      > if they were one function before they probably share some state and this is exactly why you refactor to pull out the shared state into parameters, so that each of the "subfunctions" have zero side effects.
    • nojokes 5y ago
      Now this is usually in my opinion not a good advice (it is like reintroduction of global variables) as unnecessary state certainly makes things more difficult to reason about. I have read the book (not very recently) and I do not recall this but perhaps I am just immune to such advice. I like his book about refactoring more than Clean Code but it introduced me to some good principles like SOLID (a good mnemonic), so I found it somewhat useful.
  • jollybean 5y ago
    Yes and no. What I find is that function boundaries have a bunch of hidden assumptions we don't think about. Especially things like exceptions. For all these utility functions are you going to check input variables, which means doing it over, over and over again. Catching exceptions everywhere etc? A function can be used for a 'narrow use case' - but - when it's actually made available to other parts of the system, it needs to be kind of more generalized. This is the problem. Is it possible that 'nested functions' could provide a solution? As in, you only call the function once, in the context of some other function, so why not physically put it there? I can have it's own stack, be tested separately if needed, but it remains exclusive to the context that it is in from a readability perspective - and you don't risk having it used for 'other things'. You could even have an editor 'collapse' the function into a single line of code, to make the longer algorithm more readable.
    • JamesBarney 5y ago
      The problem is abstraction isn't free. Sometimes it frees up your brain from unnecessary details and sometimes the implementation matters or the abstraction leaks. Even something as simple as Substring which is a method we use all the time and is far more clear than most helper functions I've seen in code bases. Is it Substring(string, index, length) or Substring(string, indexStart, indexEnd) What happens when you pass in "abc".Substring(0,4) do you get an exception or "abc"? What does Substring(0,-1) do? or Substring (-2,-3). What happens when you call it on null? Sometimes this matters, sometimes it doesn't.
      • TeMPOraL 5y ago
        Also: - Does it destructively modify the argument, or return a substring? Or both? - If it returns a substring, is it a view over the original string, or a fresh substring that doesn't share memory with the original? - If it returns a fresh substring, how does it do it? Is it smart or dumb about allocations? This almost never matters, except when it does. - How does it handle multibyte characters? Do locales impact it in any way? With the languages we have today, a big part of the function contract cannot be explicitly expressed in function signatures. And it only gets worse with more complicated tools of abstraction.
• nojokes 5y ago
  I am surprised that this is the top answer (Edit: at the moment, was) How does splitting code into multiple functions suddenly change the order of the code? I would expect that these functions would be still called in a very specific order. And sometimes it does not even make sense to keep this order. But here is a little example (in a made up pseudo code): function positiveInt calcMeaningOfLife(positiveInt[] values) positiveInt total = 0 positiveInt max = 0 for (positiveInti=0; i < values.length; i++) total = total + values[i] max = values[i] > max ? values[i] : max return total - max ===> function positiveInt max(positiveInt[] values) positiveInt max = 0 for (positiveInt i=0; i < values.length; i++) max = values[i] > max ? values[i] : max return max function positiveInt total(positiveInt[] values) positiveInt total = 0 for (positiveInt i=0; i < values.length; i++) total = total + values[i] return total function positiveInt calcMeaningOfLife(positiveInt[] values) return total(values)-max(values) Better? No?
  • viktree 5y ago
    > How does splitting code into multiple functions suddenly change the order of the code? Regardless of how smart your compiler is and all the tricks it pulls to execute the codein much the same order, the order in which humans read the pseudo code is changed 01. function positiveInt max(positiveInt[] values) 02. positiveInt max = 0 03. for (positiveInt i=0; i < values.length; i++) 04. max = values[i] > max ? values[i] : max 05. return max 07. function positiveInt total(positiveInt[] values) 08. positiveInt total = 0 09. for (positiveInt i=0; i < values.length; i++) 10. total = total + values[i] 11. return total 12. function positiveInt calcMeaningOfLife(positiveInt[] values) 13. return total(values) - max(values) Your modern compiler will take care of order in which the code is executed, but as humans need to trace the code line-by-line as [13, 12, 01, 02, 03, 04, 05, 07, 08, 09, 10, 11]. By comparison, the inline case can be understood sequentially by reading lines 01 to 07 in order. 01. function positiveInt calcMeaningOfLife(positiveInt[] values) 02. positiveInt total = 0 03. positiveInt max = 0 04. for (positiveInt i=0; i < values.length; i++) 05. total = total + values[i] 06. max = values[i] > max ? values[i] : max 07. return total - max > Better? No? In most cases, yeah probably your better off with the two helper functions. max() and total() are common enough operations, and they are named well enough that we can easily guess their intent without having to read the function body. However, depending on the size of the codebase, the complexity of the surrounding functions and the location of the two helper functions it's easy to see that this might not always be the case. If you want to try and understand the code for the first time, or if you are trying to trace down some complex bug there's a chance having all the code inline would help you. Further, splitting up a large inline function is more trivial than reassembling many small functions (hope you got your unit tests!). > And sometimes it does not even make sense to keep this order. Agreed. But naming and abstractions are not trival problems. Often times it's the larger/more complex codebases, where you see these practices get applied more dogmatically
• joshka 5y ago
  >If you split it into 10 functions 10-lines-long each now you have 10! possible orderings of calling these functions (ignoring loops and branches). And since this ordering is separated into multiple places - you have to keep it in your mind. Good luck inventing naming that will make obvious which of the 3628800 possible orderings is happening without reading through them. It's easy to make this argument in the abstract, but harder to demonstrate with a concrete example. Do you happen to have any 100 lines of code that you could provide that would show this as a challenge to compare to the refactored code? You're missing likely missing one or more techniques that make this work well: 1. Depth first function ordering, so the execution order of the lines in the function is fairly similar to that of the expanded 100 lines. This makes top to bottom readability reasonable. 2. Explicit naming of the functions to make it clear what they do, not just part1(); part2() etc. 3. Similar levels of abstraction in each function (e.g. not having both a for loop, several if statements based on variables defined in the funtion, and 3 method calls, instead having 4-5 method calls doing the same thing). 4. Explicit pre/post conditions in each method are called out due to the passing in of parameters and the return values. This more effectively helps a reader understand the lifecycle of relevant variables etc. In your example of 100 lines, the counterpoint is that now I have a method that has at least 100 ways it could work / fail. By breaking that up, I have the ability to reason about each use case / failure mode.
  • deleted 5y ago
    [deleted]
  • deleted 5y ago
    [deleted]
  • ajuc 5y ago
    > You're missing likely missing one or more techniques that make this work well: I know how to do it, I just don't always think it's worth it. > Do you happen to have any 100 lines of code that you could provide that would show this as a challenge to compare to the refactored code? Not 100 lines, just 34, but it's a good example of a function I wouldn't split even if it get to 300 lines. function getFullParameters() { const result = { "gridType": { defaultValue: 1, randomFn: null, redraw: onlyOneRedraw("grid"), }, "gridSize": { defaultValue: 32, randomFn: null, redraw: onlyOneRedraw("grid"), }, "gridOpacity": { defaultValue: 40, randomFn: null, redraw: onlyOneRedraw("grid"), }, "width": { defaultValue: 1024, randomFn: null, redraw: allRedraws(), }, "height": { defaultValue: 1024, randomFn: null, redraw: allRedraws(), }, "seed": { defaultValue: 1, randomFn: () => Math.round(Math.random() * 65536), redraw: allRedraws(), }, "treeDensity": { defaultValue: 40, randomFn: () => Math.round(Math.random() * 100), redraw: onlyOneRedraw("trees"), }, "stoneDensity": { defaultValue: 40, randomFn: () => Math.round(Math.random() * 20 * Math.random() * 5), redraw: onlyOneRedraw("stones"), }, "twigsDensity": { defaultValue: 40, randomFn: () => Math.round(Math.random() * 20 * Math.random() * 5), redraw: onlyOneRedraw("twigs"), }, "riverSize": { defaultValue: 3, randomFn: () => Math.random() > 0.5 ? Math.round(Math.random() * 10) : 0, redraw: onlyRedrawsAfter("river"), }, "roadSize": { defaultValue: 0, randomFn: () => Math.random() > 0.5 ? Math.round(Math.random() * 10) : 0, redraw: onlyRedrawsAfter("river"), }, "centerRandomness": { defaultValue: 20, randomFn: () => Math.round(30), redraw: onlyOneRedraw("trees"), }, "leavedTreeProportion": { defaultValue: 95, randomFn: () => Math.round(Math.random() * 100), redraw: onlyOneRedraw("trees"), }, "treeSize": { defaultValue: 50, randomFn: () => Math.round(30) + Math.round(Math.random() * 40), redraw: onlyOneRedraw("trees"), }, "treeColor": { defaultValue: 120, randomFn: () => Math.round(Math.random() * 65536), redraw: onlyOneRedraw("trees"), }, "treeSeparation": { defaultValue: 40, randomFn: () => Math.round(80 + Math.random() * 20), redraw: onlyOneRedraw("trees"), }, "serrationAmplitude": { defaultValue: 130, randomFn: () => Math.round(80 + Math.random() * 40), redraw: onlyOneRedraw("trees"), }, "serrationFrequency": { defaultValue: 30, randomFn: () => Math.round(80 + Math.random() * 40), redraw: onlyOneRedraw("trees"), }, "serrationRandomness": { defaultValue: 250, randomFn: () => Math.round(100), redraw: onlyOneRedraw("trees"), }, "colorRandomness": { defaultValue: 30, randomFn: () => Math.round(20), redraw: onlyOneRedraw("trees"), }, "clearings": { defaultValue: 9, randomFn: () => Math.round(3 + Math.random() * 10), redraw: onlyRedrawsAfter("clearings"), }, "clearingSize": { defaultValue: 30, randomFn: () => Math.round(30 + Math.random() * 20), redraw: onlyRedrawsAfter("clearings"), }, "treeSteps": { defaultValue: 2, randomFn: () => Math.round(3 + Math.random() * 2), redraw: onlyOneRedraw("trees"), }, "backgroundNo": { defaultValue: 1, randomFn: null, redraw: onlyTheseRedraws(["background", "backgroundCover"]), }, "showColliders": { defaultValue: 0, randomFn: null, redraw: onlyOneRedraw("colliders"), }, "grassLength": { defaultValue: 85, randomFn: () => Math.round(25 + Math.random() * 50), redraw: onlyTheseRedraws(["background", "backgroundCover"]), }, "grassDensity": { defaultValue: 120, randomFn: () => Math.round(25 + Math.random() * 50), redraw: onlyTheseRedraws(["background", "backgroundCover"]), }, "grassSpread": { defaultValue: 45, randomFn: () => Math.round(5 + Math.random() * 25), redraw: onlyTheseRedraws(["background", "backgroundCover"]), }, "autoredraw": { defaultValue: true, randomFn: null, redraw: noneRedraws(), }, }; return result; } There's a lot of value in having all of this in one place. Ordering isn't a problem here, just no need to refactor.