19 ms·
In the Defense of Spaghetti Code (2013)
- SenHeng 7y ago(2013)
- wodenokoto 7y agoLots of small functions and special classes being called left and right can be much more difficult to follow than just the whole logic written out consecutive.
- reallydontask 7y agoIt's all about balance really and this is part of the problem. 1500 lines of code for a function/method would strongly suggest a problem. In the same way as a 200 function/methods would make it hard to follow.
- barking 7y agoIf breaking up a procedure results in the creation of a bunch of smaller procedures that are only called once then I think I'd prefer to leave it as it was with well demarcated and commented sub-sections. It saves you the job of inventing descriptive procedure names as well as jumping between procedures. I seem to recall also that in Code Complete, Steve McConnell claimed that there was an inverse relation between the length of a procedure and the number of errors per line of code.
- CuriouslyC 7y agoHow is an inline function different from a demarcated and commented subsection? It seems to me that the semantic and enforced correctness benefits of functions would make using them in this way a no brainer.
- noxToken 7y agoI got into this exact argument a while ago. We have an informal, internal audit of our code base to highlight problem areas, trends, and note if something we're doing is problematic over time. Someone suggested a hard cap of 30 lines per function. I immediately had a rebuttal with code. We have some functions that are mostly mappers. Ingest code from one place, transform/cast/sanitize/clean it, then send it elsewhere. The types of information for some of these functions can make some of these behemoths 200+ lines. I asked my peer if they would rather follow the trail back-and-forth of 20+ helper functions or look at one monolith knowing that it only maps data? They wanted helper functions. To counter, I highlighted a mistake that I made years ago. I traced the revision of a file where a function mapped data that used helper functions. I pointed out that I did the exact thing suggested. There were many helper functions of 3-10 lines mapping data and returning back to a single place in a file. I'm not that smart. Rather, I've done a lot of academic, clever, and sometimes outright dumb things, I haven't forgotten those lessons learned.
- CuriouslyC 7y agoDiving into and out of functions can make the code harder to debug at first, and it can slow down how long it takes to figure out exactly what the code is doing on a micro level. On the other hand, it greatly simplifies the process the process of understanding what a function is doing at a high level. It also makes it much easier to navigate the code quickly and determine exactly where changes need to occur.
- diminoten 7y agoIf it's more difficult, then the small functions and classes can be improved. Alternatively, maybe you need to brush up on how you read code. If you need a whole program laid out consecutive for you to understand it, that's not great.
- Barrin92 7y agoAgreed. Factoring out code to functions when those functions aren't used anywhere else to me just seems like adding an additional level of indirection that makes understanding the code more complicated, because now you have to jump between the purely declarative names and the code when in fact you're trying to follow control flow.
- kd5bjo 7y agoThe article’s argument is a reductio ad absurdum that flows from the outcome of the purportedly reasonable refactoring of this: int new_sequence_number (int old_sqn, int foosoft) { if (foosoft == 2) // version 5.34.2 of the protocol return old_sqn + 3; else if (foosoft) return old_sqn + 2; else return old_sqn + 1; } into this: int new_sequence_number (int old_sqn, int step) { return old_sqn + 1 + step; } The problem is that this isn’t reasonable. Though the function signature looks the same, foosoft is an enumeration of some kind but step is actually an integer— This sort of change requires an audit of all call sites, as the meaning of one of the arguments has changed. One big red flag here is that the new function body isn’t what you’d expect from its signature. Why is the sequence number incremented by step+1 instead of step? The only reason I can think of is that they want to maintain compatibility with an old callsite, but that doesn’t make sense with the category change of the argument. Ideally, the function signature itself should change so that the compiler will point out all of the obsolete usages to you. If you can’t do that with the type system, change the function name.
- wycy 7y agoThat the refactored version tries to perform math using the value of an enumeration also makes this feel unrealistically absurd.
- theon144 7y agoWhat you're attacking is IMHO just a part of a side-note of the actual argument; sure, that's a bad way to refactor the function. But even with an appropriate refactor, the codebase as a (fragmented) whole remains less readable and maintainable than a long spaghetti function potentially is. Because the problems now cut across responsibilities of the components built on certain assumptions made for the sake of removing the spaghetti. Because now you need to account for "global" parameters (protocol version) in lots of disparate "local" scopes - individual functions who are quickly losing their degree of encapsulation. We're discussing a hypothetical code-base, so focusing on the details isn't probably going to be too helpful. In any case however, I in no way see how the argument "flows from the outcome" of the refactor you're focusing on.
- mattmanser 7y agoIn this example the problem is the spaghetti code in the specification, which you have no control over. This then inevitably causes the resulting spaghetti code.
- UK-Al05 7y agoYou can however nicely isolate the code which has behaviour changes for each vendor.
- AstralStorm 7y agoTypically resulting in ravioli code instead. Or an explicit state machine, even worse to follow. https://en.wikipedia.org/wiki/Spaghetti_code#Ravioli_code https://en.wikipedia.org/wiki/Spaghetti_code#Ravioli_code (Wanted to link C2 wiki but it's a goner.) You cannot fix essential complexity with any kind of design. If there are options in design, your code will reflect them at some layer. You might want to suggest, say, Component or entity-based or other design, or mixins, or other object level patches for conditionals. Spaghetti if otherwise well structured is quite understandable. The more advanced versions? Not nearly. Spaghetti is explicit at least.
- kd5bjo 7y ago> You cannot fix essential complexity with any kind of design. If there are options in design, your code will reflect them at some layer. That’s certainly true; my favorite method at the moment for this sort of thing is to make it data-driven: have a structure that contains all of the parameters that change based on vendor (including enums/flags for optional behaviors) and a single process implementation that refers to it when needed.
- AstralStorm 7y agoYou end up with a big if-then block. Essentially spaghetti but localized. A joke I sometimes make is that spaghetti is best of a plate or in a box from which it cannot escape. Otherwise it would spill. Ravioli does not spill but the relationships become implicit.
- watwut 7y agoI don't think the article proves what it claims to prove. Original modification was reasonable and no abdomination. The second so called beautification, the one that expects the caller to guess step size, was definitely not improvement. More importantly, author seems to just give up around > After few such iterations the code that was originally nice and manageable becomes a mess of short functions plagued by lots of behavior-modifying arguments, the clean boundaries between components and layers are slowly dissolved and nobody is sure anymore what effect any particular change may have. Yes, that is how spaghetti code happen. But, if all those conditions were embedded in a single 1500 lines long function, the situation would be no better. You would have interconnected monstrous system of conditions.
- iraldir 7y agoI understand the author origin point: To an extent, you cannot simplify an already complex domain problem with code. However, this has nothing to do with Spaghetti code. You could have one 1500 lines function that is not spaghetti code. It's hard to imagine, but it's possible. Similarly you can definitely have a bunch of functions, classes etc. that are spaghetti code, even if they are 3 lines each. Spaghetti is about the order in which you make your calls, the convoluteness of your loop etc. I think the only point of the article is "Don't try to retro engineer the logic behind a complex domain that you do not master".
- mannykannot 7y agoYes - spaghetti code adds a dimension of complexity that is distinct from mere size, and in fact spaghetti code might be smaller than the same algorithm written as structured code (by the way, large blocks of structured code can be imagined by thinking of taking good code and inlining all the functions, with variable renaming to avoid clashes.) It is fortunate that it is possible to show, via Turing completeness, that spaghetti code cannot compute anything that structured code cannot, for if not, we would still have programmers insisting (as some did, in response to 'Goto Considered Harmful') that spaghetti code is sometimes necessary. > I think the only point of the article is "Don't try to retro engineer the logic behind a complex domain that you do not master". AKA Chesterton's Fence: https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence
- kjeetgill 7y agoI agree with the first of what you're saying but just wanted to add to others: Structured Code was a software engineer paradigm that didn't just mean "well organized" aka structured well; it was about specific constructs and conventions. Just as Functional Programming isn't just about having functions or being functional as in useful. Also, I always think Turing Completeness is a red herring in conversations about Software Engineering. Turing Completeness is such a low bar, that it hardly factors into most conversations about the expression problem. It's most notable when the goal is to actively restrict completeness. It's kinda like starting a conversation about birds and first establishing they have mass. https://en.m.wikipedia.org/wiki/Expression_problem https://en.m.wikipedia.org/wiki/Expression_problem
- TotempaaltJ 7y ago> PS. If you happen to be a developer of a static analysis tool: Consider leaving long functions alone, rather then reporting them as suspicious. There are circumstances where long functions are perfectly legal and desirable. Sure, _there are_ circumstances. They're just extremely rare and their occurrence is dependent on the team as well. So generally, I'd say function length a pretty good proxy for complexity--as long as there are overrides.
- dagss 7y agoI disagree with this. I think something should be a seperate function only if it is (or likely will be) called from several callsites. If you try to not always think about contrived cases while writing code, then this rule can sometimes lead to quite long functions simply from executing steps in sequence ("script"). One can split what is really a long function into several small ones for readability, but if one does so I think one should declare that function within the calling function (in languages that support this), and whether one does that or not is really about as insignificant as tab vs spaces or where one likes to have curly braces. My point is, if a function only has one callsite, and is not likely to get more (incl. test code), then whether to inline that function or write it seperately is a style/formatting issue, not a maintainability issue.
- kaybe 7y agoEven if you have a few callsites and just copy-paste the same function it can be readable - I've heard that called un-cooked spaghetti code. Everything is still nicely layed out and can be refactored without too much trouble if you so chose. The problem starts with cooked spaghetti code where everything is all mangled up and references are all over the place.
- barking 7y agoI don't know if I'm buying it or not but I like this cooked/uncooked spaghetti code distinction
- RHSeeger 7y ago
- theon144 7y agoKey takeaway is this: >The point I am trying to make in this article is that sometimes it's the problem domain itself that's complex and convoluted. In short, it's a spaghetti domain. I definitely have encountered spaghetti domains and have bashed my head against them, trying to find nice, clean and elegant abstractions that describe the problem succinctly and clearly. Sometimes there's just more exceptions than rules, and spaghetti code is not a bad outcome. As with all guidelines, do not follow blindly where a less "conventional" solution makes sense?
- barking 7y agoI agree, spaghetti domain seems to be a useful concept. It allows you to just get on with coding a solution.
- jstewartmobile 7y agoThere is a fine line between DRY and human-Huffman-encoder. Just knowing that the line exists makes a big difference!
- galangalalgol 7y agoI'm stealing that analogy, it's perfect. Huffman encoded codebases give me rage tantrums when debugging.
- kartan 7y agoYes. Over-engineering is a thing. Also, there are problems that cannot be simplified (e.g. Implementing a Calendar class http://yourcalendricalfallacyis.com/ http://yourcalendricalfallacyis.com/ ) I agree with other comments that Spaghetti Code is not the right term for this. But, the original point is valid. The other part of the point is also true. To hide implementation inside functions makes sense if the function is part of the real world domain. To do a cut/paste to extract lines of codes of a big function to make it look nicer may increased complexity as now jumping around the code is needed to read it.
- deleted 7y ago[deleted]
- nickpsecurity 7y agoIf the requirements are complex, then you list the requirements, what you're doing with each one, how they connect, and the code reflects that. Breaking it into functions and modules for that often does enough. At the least, the design documentation will facilitate understanding even if the code itself is harder to understand. Spaghetti code is almost always just bad design and coding.
- hzhou321 7y agoA refactored code and the spaghetti version are two sides of the same code. Just like any complex object requires multiple viewpoints to comprehend, asserting one perspective is superior to the other is missing the picture. It is a pity that modern practice is always one or the other. It is possible to have both. Literate programming is one example.
- pmontra 7y agoI don't dare to call it spaghetti code because (among the other things) it's split in a lot of functions, but to give an example that's probably equally difficult to everybody, check this Erlang module that parses ASN1 [1] A few hints about the language for the non-functional crowd, then my point 1) Functions can have multiple definitions and are picked top to bottom. The one that runs is the first one matching the arguments in the call. 2) Variables are Capitalized (and can't be assigned again, big surprise.) 3) Atoms (:symbols in other languages) are lower_cased. 4) The arrow (->), the comma (,) and the dot (.) do (more or less) what Java/C do with their { ; } characters (sorry, I have to keep it short) You can start from the bottom and go through the get_token, get_line, parse_ComponentTypeLists etc, find where they are called and see how they neatly (IMHO) replace the switch statements in other languages. Is it easier to understand? Hard to tell: if the problem is difficult the code won't be easy. Is it less tangled? I think it is. My point: some languages are messiers than others. [1] https://github.com/erlang/otp/blob/master/lib/asn1/src/asn1ct_parser2.erl https://github.com/erlang/otp/blob/master/lib/asn1/src/asn1c...
- _pmf_ 7y agoTo add to this (for those unfamiliar with Erlang/OTP): Erlang not only has general pattern matching, but also specific support for pattern matching on binary content (down to bitfields). It's better than C and C++ in that regard in more than one way.
- _bxg1 7y ago...spaghetti code isn't a monolithic function, it's a constellation of tightly-coupled but separate entities, woven together like a big knot. I.e., exactly the thing the author advises against. Terminology aside, he does make a good point: if the domain can't truly be broken down into clean parts, encapsulation can do more harm than good as you have to weave all these contingencies through every layer.
- jkingsbery 7y ago> Luckily though, 45 years after Dijkstra's paper, goto is not used extensively any more. goto might not literally be used any more, but I see "break" used a lot, and for long functions that have variables defined outside of the scope of the "break," it behaves similarly.
- analog31 7y agoIt would be interesting to go back through old code containing GOTO's, and see if the main use was similar to "break."
- contravariant 7y agoIf using something that ought to be an enum as an integer to save a few lines of code isn't spaghettification then I don't know what is. Rather predictably it doesn't generalise. It's called spaghetti because it's hard to see what information flows where, not just because some information flows are long (at least, my mental image for spaghetti-code has always been cooked spaghetti).
- bregma 7y agoI cut my coding teeth in assembly, followed by FORTRAN IV and a smattering of Apple, Commodire, and TI BASIC (with their gosubs and peek/pokes). I've worked with real spaghetti code. What most people call spaghetti code these days is just code. Useful programs are complex and have complex code. Modern software might be difficult to follow, espcially if you're at an amateur or junior level (which is probably 80% of the people out there reading code, cf. Dunning-Kreuger). It's not generally spaghetti.
- mr_crankypants 7y agoIn this particular situation, I would be much quicker to go for copy/pasted code than spaghetti code. I worked at a place where we had to interact with financial exchanges, which nominally talk standard protocols but actually all have their own quirks in their implementations of the protocols, which we had to account for on our own end. They ended up going with the copy/paste approach, and, while I was initially horrified, I came to realize that they're right. The admonishment against copy/paste is meant to deal with the risk of accidentally forgetting to update one of the copies when there's a new change that needs to be pushed across all of them. It turns out that, for the problem domain we were working in, which sounds quite similar to the one TFI describes, that never happens. Even when the standards body releases a new version of the protocol, each vendor upgrades on their own schedule, and the slowest-moving vendor might stick on the old version for 5 or even 10 years, and when they finally do get around to it, their implementation of that feature will have its own quirks, so just setting a flag saying, "Vendor X is now on version Y" wasn't going to work, anyway. So then you (hypothetically) create a set of flags saying, "Vendor X is using version Y, but with these behaviors that differ from everyone else's version of Y", add a bunch of new if-statements, and the actual logic that might execute at run-time gets even more obfuscated behind a tangle of counterfactual configurations that won't happen in real life but still exist in the code and are supported in the application. So, to sum up: nominally the same protocol, but everyone does it differently, and upgrades on their own schedule. It really is a horrendous tangle. Dealing with that mess by mirroring it in your code with an equally tangled pile of flags and conditionals is also going to be a mess, no matter how you choose to organize the code. Don't do that. Alexander the Great was right: Sometimes the best way to deal with these sorts of things is by cutting them into pieces. Then, when you're dealing with one implementation's quirks, you only have to be thinking about that one set of quirks, and not the combinatorial explosion of possible interactions that you get when you try to mix them all together.