5 ms·
I've seen this idea cargo-culted way too often. There's nothing wrong with long functions. In fact, long functions are often easier to grok than ones split apar
by pubby 7y ago
I've seen this idea cargo-culted way too often. There's nothing wrong with long functions. In fact, long functions are often easier to grok than ones split apart into dozens of pieces.
Create useful abstractions. Don't create abstractions for the sake of creating abstractions.
- bransonf 7y agoI share your sentiment. Some of the programmers I’ve worked with would build functions for every thing within a larger function. Need to chunk the data? Function. API Call? Function. Parse the response? Function. So I asked: “Are you ever going to use these internal functions for anything outside of this function?” And their response was: “No, but it makes debugging easier.” It does not. It makes it exponentially more difficult. Maybe I just work with too many terrible programmers...
- enriquto 7y agoI have the opposite philosophy. The ideal function is only called once. If it is called more than once, I have a "call diamond" instead of a "tree" and this is a code smell.
- raxxorrax 7y agoHarsh constraint. Probably difficult to come up with new sorting algorithms or substitutes for basic arithmetic functions after a while.
- enriquto 7y agoI mean for functions that are called in the same context. Obviously external and well-abstracted functions can be called several times.
- Nition 7y agoOne middleground option if you have a long function but also want to break it into chucks for easy readability: In C# and some other languages you can put braces around a block of code without actually having any condition on it. Or just throw some big comment headers in.
- skohan 7y agoIn Swift I use locally-scoped functions for this. It has the advantage of ensuring that this single-use function can't be called from outside the intended scope, but I can still chunk the work into easy-to-reason-about pieces.
- Nition 7y agoC# has those now also: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/local-functions https://docs.microsoft.com/en-us/dotnet/csharp/programming-g...
- aptwebapps 7y agoVS Code has regions, which look like preprocessor instructions but I don't think they actually have any affect on the code. They do, however collapse while providing a nice label.
- scns 7y agoJetbrains IDEs have those too
- aptwebapps 7y agoThanks, I've been using PyCharm for years and didn't know that.
- pubby 7y agoWoah, bransonf? I met you at OpenSTL a few weeks ago! What a funny coincidence. And yeah, I agree. Even Carmack had something to say on this: http://number-none.com/blow/john_carmack_on_inlined_code.html http://number-none.com/blow/john_carmack_on_inlined_code.htm...
- rustybolt 7y agoDepends. In complicated code with lots of dependencies, I've always found it easier to understand functions that are written as: function do_task(a, b, c) { d = subtask1(a, b); e = subtask2(b, c, d); f = subtask3(a, d); } instead of one long function. Of course, this only makes sense if the subtasks are indeed different conceptual steps in the computation. As an example, I remember writing finite elements code that was structured something like this: function solve_fem_problem(problem_description) { basis_functions = generate_basisfunctions(basisfunctions_type.b_spline); quadrature_rule = generate_quadrature(quadrature_type.gaussian_quadrature); A = assemble_matrix(problem_description.differential_equation, basis_functions, quadrature_rule); b = assemble_righthandside(problem_description.differential_equation, basis_functions, quadrature_rule); apply_boundary_conditions(problem_description.boundary_conditions, A, b); solution = linsolve(A, b); } which I think is very pleasing to read, and preferable to a single huge function. This also makes it easy to use another linear solver, for example. On the other hand, the functions that are called inside solve_fem_problem are probably larger. Note: During work, I haven't actually encountered a lot of code that is structured in this way.
- Symbiote 7y agoI'm hijacking this since your function shows the kind of case where I like to make an exception to the line length rule. It looks much better, and is more understandable due to the repeating pattern, as you have it, but some people I've collaborated on open source code with would insist upon this: function solve_fem_problem(problem_description) { basis_functions = generate_basisfunctions(basisfunctions_type.b_spline); quadrature_rule = generate_quadrature(quadrature_type.gaussian_quadrature); A = assemble_matrix(problem_description.differential_equation, basis_functions, quadrature_rule); b = assemble_righthandside(problem_description.differential_equation, basis_functions, quadrature_rule); apply_boundary_conditions(problem_description.boundary_conditions, A, b); solution = linsolve(A, b); } (For this example, I'm assuming an arbitrary maximum line length of between the A = and b = lines.)
- Jestar342 7y ago> It does not. It makes it exponentially more difficult. You should realise you are falling victim of your own assertions here. You're cargo-culting the idea that abstracting functions makes debugging exponentially more difficult, without having provided any substance to qualify.
- bransonf 7y agoThe assertion, which I believed was implied, is that the habit of creating many sub functions will make it more difficult to trace a bug. Imagine having a function with 5 sub functions, and since this is your style, those functions have 5 internal functions. If I want to trace an error, first I have to find it in the main function, then the sub function, and maybe even within its sub functions. Then I have to deal with several different scopes as well. I don’t mind this if a function is a useful abstraction, that is it does something well and is used repeatedly. My gripe is with functions that are written just for the sake of writing functions.
- Jestar342 7y ago> It does not. It makes it exponentially more difficult. is not implicit. That is very much an explicit assertion. I'm simply pointing out that in your pointing out of cargo-cult, you've done the exact same thing of drawing a conclusion with no basis. > Imagine having a function with 5 sub functions, and since this is your style, those functions have 5 internal functions. is unrealistic. There's plenty of room in between "no private functions at all" and "5^n functions for every layer". It's hyperbole and bordering rhetorical. As for tracing bugs in the way you mention - nobody in my sphere of influence has needed to do that in years, at least not on a regular basis. Tests demonstrating IPO are far more valuable and also far more quickly than tracing, narrow the scope of the problem to the point that you'll now be within one small function to read than a potentially much larger one.
- z3t4 7y agoWith risk of offending both sides of the argument, both long functions, and split functions are fine. Functions within functions are great for async code as you can make use of the closure/lexical scope. eg. When the network request come back you still have acces to variables declared in parent function. And you can also freeze state/value of variables in time - by making them into function parameters copied by value.
- moksly 7y agoI’m danish, like I assume the author is, and what he preaches here is very typical of meta-programming in our C# crazed nation. I get it too, it’s a lot easier to have a lot of short functions with single responsibilities in a neat structure while you’re working on a C# project. As long as you know what you did and where you put it, it’s absolute bliss. It’s also really easy to sell your clients a lot of useless automated tests for this approach. Five years later when I come in to debug the monster you’ve lost track off, however, I’ll curse you to the death as I go through endless loops of “go to definition”.
- skohan 7y agoWhat specifically about C# makes this approach appealing?
- skocznymroczny 7y agoI guess it's one of the few languages (Java is another) with proper IDE support. Most other language IDEs mostly provide syntax highlighting and basic support for debugging and go-to-definition, which also tends to break.
- moksly 7y agoVisual Studio is amazing at juggling things that are wonderful while you have a mental map of your code and nightmarish once you don’t.
- enriquto 7y ago> I've seen this idea cargo-culted way too often maybe, but it should still be more widespread
- beatgammit 7y agoThe basic ideas, sure, but not the specifics. The ideas as I see them are: - keep functions simple and single purpose, which makes them easy to understand quicky - avoid long lines of code since they don't fit uniformly on one screen - make functions as testable as is reasonable Whenever you introduce hard rules, you get weird workarounds for cases that don't fit nicely. Dogma is always bad, concepts are immortal. So yeah, if this helps teach a concept, use it, but don't force everything to adhere to a strict code style.
- bsder 7y agoI'm sooooo tired of this carve things up into functions cargo cult. Go grab a Vulkan tutorial, and try to understand the logical flow. Good luck. All the Vulkan tutorials atomize things into functions because it "encapsulates" actions into "easier to understand" pieces. And then creates a mutable uber-object that holds a whole bunch of Vulkan handles that needs to get passed to every function so you don't know who is updating what and when. But, hey, writing that as a 1500 line single function where you only expose the variables as you need them is far too complex for us mere mortals to read. <rolls eyes>
- thrower123 7y agoSo, they're advising to recreate a custom version of the old OpenGL statemachine?
- bsder 7y agoThat's the result, but it's not their intent. The problem is that Vulkan requires about 1500 lines of boilerplate to put a simple triangle on the screen. The issue is that you have to wire as much as possible up statically so the graphics hardware can fly once you start feeding it vertex information. However, for someone who doesn't need maximum performance, the setup boilerplate is a nuisance as Vulkan doesn't do the idea of "sensible default". So, everybody tries to "encapsulate" this boilerplate. The problem is that this obscures the fact that you are setting up things which have distinct dependency orders. And sometimes those orders are different depending upon what you are trying to do. If you already understand advanced graphics card hardware, dependency encapsulation is great. If you really don't yet have the ideas of things like subpasses, pipelines, graphics queues, and queue synchronization really crystal clear, you don't want things abstracted as you are still experimenting and prodding the code in weird ways.
- robert_tweed 7y agoIndeed, it is a fundamentally flawed principle. Code needs to be considered as a graph. All indirections within the code create an edge. Linear blocks of atomic instructions make up a node. The first fallacy is only considering the size of the nodes and ignoring the number of edges. This is like micro-optimising without paying attention to the Big-O factor. The second fallacy is failing to consider the difference between what I will call static branches and runtime branches. An if statement or a for loop are static branches. Other forms of indirection happen at runtime (with "goto" being famously difficult to analyse). Static indirection is easy to analyse, runtime indirection is hard. The code graph (I use this term to mean something slightly simpler than the call-graph, which includes things like recursion) tends to get considerably more complex at runtime. One of the best ways to make your runtime code graph more complex is to add more functions, especially ones that are not private to the current file. As soon as this happens, you have no immediate way of knowing how many runtime edges there will be going in and out of each one of those functions. So hypothetically, let's say you take a 21-line function and break it into 3, 7-line functions. While each of those is easy to "read", you still have to understand 21 lines of interdependent code, but now the total complexity has potentially increased exponentially. How bad this problem is depends on the scope of the functions (can dramatically increase how many call-sites can potentially form new runtime edges - something you cannot tell from looking at the code in front of you) and whether that code has any side-effects. Certain types of runtime indirection like DI frameworks can completely thwart attempts to statically analyse this runtime graph. If you are only dealing with pure functions then it can seem like this isn't a problem because of referential transparency. However, this ignores two problems: the name of the function must precisely describe what it does, otherwise it will mislead "distant code readers" (people reading the call but not the source). That must also never change in the future, otherwise you will still have to deal with all that extra complexity if you ever want to refactor safely. This is not to say that abstractions are bad or that long functions are good. Premature abstraction however tends to produce bad abstractions, which are worse by far than no abstraction.