19 ms·

What makes code hard to read: Visual patterns of complexity (2023)

• Pannoniae 2y ago
  This is probably a bit of a cheap dismissal.... but I think this article misses the forest for the trees. They set a standard which they manage to meet - but do not really introspect on whether the standard is appropriate or not. All of these metrics (except variable liveness) are on a method/function level. Guess what this encourages? Splitting everything into three-line methods which makes the codebase a massive pile of lasagna full of global or shared state. If a method is long to read from top to bottom, the answer isn't always splitting it into 5 smaller ones, sometimes life just has inherent complexity.
  • LandR 2y ago
    > If a method is long to read from top to bottom, the answer isn't always splitting it into 5 smaller ones, sometimes life just has inherent complexity. Yes! This. I find it much easier to parse a long function where I can scroll down it and just read it top to bottom, then having a function which calls out to lots of other functions and I'm jumping around the code base, back and forward. Just reading the long function top to bottom, where I can very easily just scroll up a bit is so much easier to keep in my head. Even worse is when you go to definition on the method and you get 5 options, and you have to figure out which one would actually get called given the current path through.
    • _dark_matter_ 2y ago
      100%. Worst is when the called function is in a separate file, and the most upsetting is when it's the _only_ function in the file. I really wish IDEs or tools like Sourcegraph could handle this better.
      • rudasn 2y ago
        What are talking about?! That's my favourite part when reading code! :P
    • dijksterhuis 2y ago
      > [than] having a function which calls out to lots of other functions and I'm jumping around the code base, back and forward. i agree with longer functions and less jumping around, but there's also some nuance i find. I sometimes find converting a complicated multi-line condition into something like the below is much easier for me to read, so long as the function is named in a clear way and the function definition is just above the big function it gets called by (never in a different file!) def is_something_valid(something, foo, bar): return something > 0 and something != foo and something > bar if is_something_valid(something, foo, bar): it's like a convenience reading shortcut so i don't have to parse the logic in my head if i don't want to. also makes the condition nice and easy to test. then again, can also do it the grug-brained way gt_zero = something > 0 ne_foo something != foo gt_bar something > bar if gt_zero and ne_foo and gt_bar:
      • jabiko 2y ago
        I think you are making a good point but if this function is only used in one place I would personally prefer to just use a variable: something_is_valid = something > 0 and something != foo and something > bar if something_is_valid: # do stuff That way you can document the intention of the condition using the variable name while at the same time keeping the locality of the check.
      • robert_dipaolo 2y ago
        I mostly agree, but for short one liners and where there will be no reuse elsewhere, instead of a function I prefer; something_is_valid = something > 0 and something != foo and something > bar if something_is_valid: # .... It achieves the same thing without needing to scroll.
      • jodrellblank 2y ago
        > can also do it the grug-brained way This way reads like: x = 1 // set variable x equal to 1 in that gt_zero echoes what the > operator does and says nothing about intent. Comparing, e.g. gt_zero = space > 0 // there is some space I guess? space_for_logfile = space > 0 // oh, logfiles need space > 20 there's the mistake.
    • mpweiher 2y ago
      I think the issue is whether the functions that are split out are actually useful abstractions. If they are, you should not have to jump around the code-base, you should be able to just read the invocation and know what it does, without leaving the source function. As an example, you probably don't whip out your kernel source code when you encounter a call to write(). At least not usually. You just know what it does and can keep going. You probably also don't look at the generated assembly code, and maybe look up the instruction reference for your favorite microprocessor when you encounter an arithmetic operator. You just assume that you know what it does, even if that may not be 100% correct in every case. Those are good, useful abstractions. That's what we need to strive for when we crate code.
    • jghn 2y ago
      For longer functions vs bouncing between smaller functions my experience has been that this is one of those things where people are one way or the other. And they almost never change their preference. If your coworkers are all the same as you, that's great. If they're not, prepare for battle.
  • deleted 2y ago
    [deleted]
  • hobs 2y ago
    I would use the term irreducible complexity - you can move it around but you cant get rid of it, and spreading it all over your code smoothly and evenly makes it 10x harder to change or reason about it.
  • michaelcampbell 2y ago
    > the answer isn't always splitting it into 5 smaller ones To someone who just read a book about it, it is. I've heard this called "rabbit hole" programming; it's function after function after function, with no apparent reason for them other than the line count. It's maddening.
    • LandR 2y ago
      I think the whole Uncle Bob clean code movement has a lot to answer for.
      • bluGill 2y ago
        While then Uncle Bob clean code movement has a lot of answer for, it is far far better than much of what came before. I've had to work with 60,000 line functions where #ifdefs worked across brace boundaries ... #ifndef foo break; case SomeCondition: doSomething(). #endif moreCode(); break; I'll take the worse uncle Bob can throw at me over that mess.
      • dsego 2y ago
        https://github.com/johnousterhout/aposd-vs-clean-code https://github.com/johnousterhout/aposd-vs-clean-code
  • cardanome 2y ago
    While I think that there is no harm in longer functions as long as the code is linear, I think the the problem is that people abstract the wrong way. The main issue I see is people writing functions that call functions that call functions. No. Don't. Have one main function that is in control and that calls all the sub-functions. Sub-functions do not call functions on their same level, ever. Yes, those sub-function could have their own helper function and so on but the flow always needs to be clearly one-directional. Don't go across and create a spaghetti mess. Trees not graphs. Keep your code structure as flat as possible. > massive pile of lasagna full of global or shared state. Yeah, the skill is to avoid shared state as much as possible. Handling state centrally and opting for pure functions as much as possible.
• jt2190 2y ago
  Kudos to seeinglogic for trying to quantify that “readablity” is. We need a lot more of this. (I feel like the most common definition of readability in use today is “readable to me“.) I have a half-baked thought that we could find the actual dimensions of readability if we gave a test to a very large group of people and asked them to pick a sentence that describes what the code does. Each question would be timed. The questions that most people answered correctly in the shortest average time would provide us with examples of “real-world readable” code, and more importantly, might help us identify some truly not-readable practices. I predict we’d see respondents start to cluster around various things, like “how long have they been programming?“, “do they understand programming paradigm X?“, etc. Perhaps the results would shift over time, as various things came into and out of fashion.
  • usrbinenv 2y ago
    That's very one-dimensional. It's usually easy to tell what the code does, but what's hard is to modify or add functionality to it. And this is because of various levels of abstractions that hide how things are interconnected.
    • usrbinenv 2y ago
      Another thing that comes to mind is the level at which one is familiar with a particular style of code organization & a set of abstractions. For example, Rails devs really have absolutely no problem getting up to speed with any given Rails app, but people who don't practice Ruby/Rails as their primary language/framework often complain how complicated and magical it is.
    • esafak 2y ago
      Poor abstractions. Good abstractions make it easier to change things, by decomposing the code into cohesive pieces with low coupling so that you can swap them out and having to think about the surrounding pieces beyond their interfaces. A good interface is small and logical.
      • bluGill 2y ago
• BorgHunter 2y ago
  I think things like Halstead complexity or cyclomatic complexity are more heuristic than law. To read code, the most important thing to me is the abstractions that are built, and how effectively they bury irrelevant complexities and convey important concepts. As an example, I recently refactored some Java code that was calling a service that returned a list of Things, but it was paged: You might have to make multiple calls to the service to get all the Things back. The original code used a while loop to build a list, and later in the same function did some business logic on the Things. My refactoring actually made things more complex: I created a class called a Spliterator that iterated through each page, and when it was exhausted, called the service again to get the next one. The upside was, this allowed me to simply put the Things in a Stream<Thing> and, crucially, buried the paged nature of the request one level deeper. My reasoning is that separating an implementation detail (the request being paged) from the business logic makes the code easier to read, even if static code analysis would rate the code as slightly more complex. Also, the code that handles the pages is fairly robust and probably doesn't need to be the focus of developer attention very often, if ever, while the code that handles the business logic is much more likely to need changes from time to time. As programmers, we have to deal with a very long chain of abstractions, from CPU instructions to network calls to high-falutin' language concepts all the way up to whatever business logic we're trying to implement. Along the way, we build our own abstractions. We have to take care that the abstractions we build benefit our future selves. Complexity measures can help measure this, but we have to remember that these measures are subordinate to the actual goal of code, which is communicating a complex series of rules and instructions to two very different audiences: The compiler/interpreter/VM/whatever, and our fellow programmers (often, our future selves who have forgotten half of the details about this code). We have to build high-quality abstractions to meet the needs of those two audiences, but static code analysis is only part of the puzzle.
  • WillAdams 2y ago
    That is exactly what is discussed in: https://www.goodreads.com/book/show/39996759-a-philosophy-of-software-design https://www.goodreads.com/book/show/39996759-a-philosophy-of...
    • 2y ago
• bluGill 2y ago
  Towards the end he had an example of splitting a sequence of "graph.nodes(`node[name = ${name}]`).connected().nodes().not('.hidden').data('name');" adding variable between some of the . in there and claimed it was marginally less efficient. This is sometimes true, but if it ever is you need to talk to your tool vendors about a better optimizes. If you are working in a language without an optimizer than the marginal difference from that optimization applied by hand will be far smaller than the performance improvements you will get by rewriting in a language with an optimizer. Either way, readability trumps performance: either because the performance is the same, or if performance mattered you would have choosen a different language in the first place.
  • isleyaardvark 2y ago
    I’d add that I follow that approach because the optimizers are more likely to optimize readable code than weird hacks.
  • evnp 2y ago
    I found this example troubling because once all the line noise is added, first-op second-op third-op fourth-op fifth-op sixth-op feels so much more impenetrable than - first-op - second-op - third-op - fourth-op - fifth-op - sixth-op The point of functional styles isn't purely brevity (as implied by the commentary around this example), it also puts a focus on the clear sequence of operations and helps reduce "operators and operands" beneficially as discussed early in the post. In general I found the post oddly dismissive of these styles, instead of weighing tradeoffs as I would hope.
    • syklemil 2y ago
      IME what we want is generally for the code to be close to the left margin and flow predictably downwards. The example with intermediate values has a lot more value to me for complex instantiations, where we can avoid nesting like it's json or yaml by using some helper variables. That problem is fundamentally the same as with deeply nested if/while/try/etc: It gets hard to visually tell what's in which scope. (Rainbow indent guides help, but they're still mitigation for a situation that can be eliminated.) But completely linear dot chains? They're fine.
• James_K 2y ago
  I my view, code complexity is best expressed in the size of it's syntax tree, with maybe an additional term for the number of unique nodes. The real mistake here is the assumption that local reductions in complexity make a meaningful difference to overall complexity. Small local decreases in complexity may guide you towards the local minimum of complexity, but will never substantially change the complexity of the code-base overall. All measurements of code complexity are essentially as good as asking "how much code do you have".
  • bluGill 2y ago
    That is ultimately by problem with the article. It isn't a bad investigation but it cannot stand alone. I never review on function in isolation. It always needs to be in context of what calls it (and often what it calls).
• WillAdams 2y ago
  Why not apply a programming methodology which allows one to leverage a rich set of tools which were created for making text more readable and visually pleasing? http://literateprogramming.com/ http://literateprogramming.com/ For a book-length discussion of this see: https://www.goodreads.com/book/show/39996759-a-philosophy-of-software-design https://www.goodreads.com/book/show/39996759-a-philosophy-of... previously discussed here at: https://news.ycombinator.com/item?id=27686818 https://news.ycombinator.com/item?id=27686818 (and other times in a more passing mention --- "Clean Code" adherents should see: https://news.ycombinator.com/item?id=43166362 https://news.ycombinator.com/item?id=43166362 ) That said, I would be _very_ interested in an editor/display tool which would show the "liveness" (lifespan?) of a variable.
  • Chris_Newton 2y ago
    I find Literate Programming interesting partly because it’s almost the opposite of the much-advocated “many small functions” style. You could literally be writing a book that explains your program, and the code becomes almost secondary material to illustrate the main text rather than the main asset itself. I did once write a moderately substantial application as a literate Haskell program. I found that the pros and cons of the style were quite different to a lot of more popular/conventional coding styles. More recently, I see an interesting parallel between a literate program written by a developer and the output log of a developer working with one of the code generator AIs. In both cases, the style can work well to convey what the code is doing and why, like good code comments but scaled up to longer explanations of longer parts of the code. In both cases, I also question how well the approach would continue to scale up to much larger code bases with more developers concurrently working on them. The problems you need to solve writing “good code” at the scale of hundreds or maybe a few thousand lines are often different to the problems you need to solve coordinating multiple development teams working on applications built from many thousands or millions of lines of code. Good solutions to the first set of problems are necessary at any scale but probably insufficient to solve the second set of problems at larger scales on their own.
    • WillAdams 2y ago
      The Axiom computer algebra folks seem to manage well --- I'm pretty sure that's the largest publicly available literate program which is available for inspection. I've been working to maintain a list of Literate Programs which have been published (as well as books about the process): https://www.goodreads.com/review/list/21394355-william-adams?ref=nav_mybooks&shelf=literateprograms https://www.goodreads.com/review/list/21394355-william-adams... I'd be glad of any I missed, or other links to literate programs. The list of projects so tagged on Github may be of interest: https://github.com/topics/literate-programming https://github.com/topics/literate-programming
• recursivedoubts 2y ago
  There is a (large, I believe) aspect of good code that is fundamentally qualitative & almost literary. This annoys a lot of computer programmers (and academics) who are inclined to the mathematical mindset and want quantitative answers instead. I love dostoyevsky and wodehouse, both wrote very well, but also very differently. While I don't think coding is quite that open a playing field, I have worked on good code bases that feel very different qualitatively. It often takes me a while to "get" the style of a code base, just as a new author make take a while for me to get.
  • User23 2y ago
    Mathematicians have recognized the importance of elegance for millennia.
  • zwnow 2y ago
    I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. Doesn't matter how it looks. If its not possible to understand what a function accomplishes within a reasonable amount of time (without requiring hours upon hours of development experience), it's simply bad.
    • ajross 2y ago
      > I consider code bad if it takes more then 5 seconds to read and understand the high level goal of a function. That's something that's possible only for fairly trivial logic, though. Real code needs to be built on an internal "language" reflecting its invariants and data model and that's not something you can see with a microscope. IMHO obsessive attention to microscope qualities (endless style nitpicking in code review, demands to run clang-format or whatever the tool du jour is on all submissions, style guides that limit function length, etc...) hurts and doesn't help. Good code, as the grandparent points out, is a heuristic quality and not one well-defined by rules like yours.
      • zwnow 2y ago
        I even wrote no matter how it looks? I meant the goal of your function needs to be grasped within a reasonable amount of time. This works for every codebase.
• mannykannot 2y ago
  I would like to add something to the point made here: "For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using a well-named variable or helper function can go a long way in reducing the cognitive load for readers. [my emphasis] // which is easier and faster to read? function funcA(graph) { return graph.nodes(`node[name = ${name}]`) .connected() .nodes() .not('.hidden') .data('name'); } // or: function funcB(graph) { const targetNode = graph.nodes(`node[name = ${name}]`) const neighborNodes = targetNode.connected().nodes(); const visibleNames = neighborNodes.not('.hidden').data('name') return visibleNames; } The names of the functions being called are rather generic, which is appropriate and unavoidable, given that the functions they compute are themselves rather generic. By assigning their returned values to consts, we are giving ourselves the opportunity to label these computations with a hint to their specific purpose in this particular code. In general, I'm not a fan of the notion that code can always be self-documenting, but here is a case where one version is capable of being more self-documenting than the other.
  • crazygringo 2y ago
    I'm almost always a fan of more rather than less commenting and self-documentation... ...but in this example I find the first to be far faster and easier to read. The "labeled" versions don't add any information that isn't obvious from the function names in the first. If you were giving business logic names rather than generic names (e.g. "msgRecipient", "recipientFriends", "visibleFriends" then I could see more value. But even then, I would find the following the easiest: function funcA(graph) { return (graph .nodes(`node[name = ${name}]`) // message recipient .connected() // recipient friends .nodes() .not('.hidden') // inside current scroll area .data('name') ); } This keeps the code itself simple and obvious, prevents a ton of repetition of variable names, and allows for longer descriptions than you'd want in a variable name.
    • mannykannot 2y ago
      You are right, using comments is even more effective in going from the generic to the specific - but then, there's a vocal minority who insist that comments are not only unnecessary, but a clear indication that you are doing it wrong. I must admit that I doubt many of them would endorse the use of auxiliary consts in the manner of the original example, either.
• deadbabe 2y ago
  You really should try to pack an unbroken thought as a single line of code as much as possible. That’s the idea behind chaining multiple functions together on one line instead of spreading it out over several lines. Eyes go horizontally more naturally than up and down, it fits our vision’s natural aspect ratio. And stop making deep nestings. Making a single function call per line assigning output to a variable each time is really just for noobs who don’t have great code comprehension skills and appreciate the pause to have a chance to think. If the variable’s purpose for existence is just to get passed on to a next function immediately, it shouldn’t exist at all. Learn to lay pipe.
  • __mharrison__ 2y ago
    I agree with you on piping but writing each method on its own line makes the code very approachable (also easier to work with). Consider this code (from a course I'm teaching this week): (df .pipe(lambda df_: print(df_.columns) or df_) .groupby('activity_id', observed=True) [non_agg_cols] .apply(lambda g: g.assign(distance=calculate_distance_np(g)), include_groups=True) .pipe(fix_index) .pipe(lambda df_: print('DONE!') or df_) ) vs: (df.pipe(lambda df_: print(df_.columns) or df_).groupby('activity_id', observed=True) [non_agg_cols].apply(lambda g: g.assign(distance=calculate_distance_np(g)), include_groups=True).pipe(fix_index).pipe(lambda df_: print('DONE!') or df_))
    • deadbabe 2y ago
      I don’t really think the first example is easier to read, it’s more of an illusion. A skilled reader should be able to carry the context as they read along. It is only because we occasionally work with inexperienced coders that the list style is necessary. Consider: “The quick brown fox jumped over the lazy ass dog” Vs The quick brown fox jumped over the lazy ass dog The second example helps a reader understand the subjects and action but it is wholly unnecessary for people who know how to read.
      • __mharrison__ 2y ago
        It is certainly easier to work with the former. If you need to comment out a line, it is painless.
• mrkeen 2y ago
  The article's good, but misses my most mentally-fatiguing issue when reading code: mutability. It is such a gift to be able to "lock in" a variable's meaning exactly once while reading a given method, and to hold it constant while reasoning about the rest of the method. Your understanding of the method should monotonically increase from 0% to 100%, without needing to mentally "restart" the method because you messed up what the loop body did to an accumulator on a particular iteration. This is the real reason why GOTOs are harmful: I don't have a hard time moving my mind's instruction-pointer around a method; I have a hard time knowing the state of mutable variables when GOTOs are in play.
  • klabb3 2y ago
    > This is the real reason why GOTOs are harmful: I don't have a hard time moving my mind's instruction-pointer around a method; I have a hard time knowing the state of mutable variables when GOTOs are in play. Well, total complexity is not only about moving the instruction pointer given a known starting point. Look at it from the callee’s pov instead of the call site. If someone can jump to a line, you can’t backtrack and see what happened before, because it could have come from anywhere. Ie you needed global program analysis, instead of local. If mutability were the true source of goto complexity then if-statements and for loops have the same issue. While I agree mutability and state directly causes complexity, I think goto was in a completely different (and harmful) category.
  • cle 2y ago
    Disagree. There's an abstract "information space" that the code is modeling, and you have to move around your mind's instruction pointer in that space. This can be helped or hindered by both mutable and immutable vars--it depends on how cleanly the code itself maps into that space. This can be a problem w/ both mutable and immutable vars. There's a slight tactical advantage to immutable vars b/c you don't have to worry about the value changing or it changing in a way that's misleading, but IME it's small and not worth adopting a "always use immutability" rule-of-thumb. Sometimes mutability makes it way easier to map into that "information space" cleanly.
• CharlieDigital 2y ago
  Maybe it's just me, but TypeScript makes code hard to read. It's fine if the data model is kept somewhat "atomic" and devs are diligent about actually declaring and documenting types (on my own projects, I'm super diligent about this). But once types start deriving from types using utility functions and then devs slack and fall back to type inference (because they skip an explicit type), it really starts to unravel because it's very hard to trace fields back to their origin in a _deep_ stack (like 4-5 levels of type indirection; some inferred, some explicit, some derived, some fields get aliased...). type Dog = { breed: string size: "lg" | "md" | "sm" // ... } type DogBreedAndSize = Pick<Dog, "breed" | "size"> function checkDogs(dogs: Dog[]) : DogBreedAndSize[] { return dogs.map(d => /* ... */) } const checkedDoggos = checkDogs([]) Versus: function checkDogs(dogs: Dog[]) { // ... } Very subtle, but for large data models with deep call stacks, the latter is completely unusable and absolutely maddening.
  • bluefirebrand 2y ago
    I agree that functions should probably specify their output type, MOSTLY to enforce that all paths that return from that function must adhere to that type I've seen plenty of regressions where someone added a new condition to a function and then returned a slightly different type than other branches did, and it broke things However, I don't think there is much value in putting types on variable declarations In your example, `const checkedDoggos = checkDogs([])` is good. Just let checkedDoggos inherit the type from the function I have a codebase I'm working on where the linter enforces `const checkedDoggos: DogBreedAndSize[] = checkDogs([])` It is very silly and doesn't add much value imo
    • CharlieDigital 2y ago
      I want it on the other side (on the function return) so that it's consistently displayed in type hints and intellisense so I don't have to navigate the code backwards 3-4 layers to find the root type (do you see what I'm saying?) function checkDogs(dogs: Dog[]) : DogBreedAndSize[] { return dogs.map(d => /* ... */) } ^^^ That's where it's important to not skip the type def because then I can see the root type in the editor hints and I don't need to dig into the call stack (I know the end result is the same whether it's on the assignment side or the declaration side, but it feels like ensuring it's always on the declaration side is where the value is)
• shortrounddev2 2y ago
  There's a cool plugin for vscode called Highlight[1] that lets you set custom regexes to apply different colors to your code. I think a common use of this is to make //TODO comments yellow, but I use it to de-emphasize logs, which add a lot of visual noise because I put them EVERYWHERE. The library I maintain uses logs that look like: this.logger?.info('Some logs here'); So I apply 0.4 opacity to it so that it kind of fades into the background. It's still visible, but at a glance, the actual business logic code pops out at you. This is my configuration for anyone who wants to modify it: //In vscode settings.json: "highlight.regexes": { "((?:this\\.)?(?:_)?logger(?:\\?)?.(debug|error|info|warn)[^\\)]*\\)\\;)": { "regexFlags": "gmi", "decorations": [{ "opacity": "0.4" }] } }, --- [1] https://marketplace.visualstudio.com/items?itemName=fabiospampinato.vscode-highlight https://marketplace.visualstudio.com/items?itemName=fabiospa...
• fs_software 2y ago
  This came up at work the other day re: client-side code readability. In one camp, folks were in favor of component extraction/isolation with self-documenting tests wherever possible. In the other camp, folks argued that drilling into a multi-file component tree makes the system harder to understand. They favor "locality of behavior". Our consensus - there needs to be a balance.
  • rob74 2y ago
    Those were exactly my thoughts while reading this article: if your codebase (over)uses inheritance, interfaces, traits, facades, dependency injection etc. etc. to thinly spread any given functionality over several files, no amount of formatting or nice naming is going to save you...
• snitzr 2y ago
  Shoutout to the pipe operator in R. The code equivalent of "and then." It helps to unnest functions and put each action on one line. I know R is more for stats and data, but I just think it's neat.
  • stared 2y ago
    Yes, dplyr pipes are wonderful. Also, for the same reason, I find JavaScript list comprehensions cleaner than those in Python - as in the former it is possible to chain maps and filters. Also, now there is a new pipe syntax in SQL, that adds a lot to readability.
    • flobosg 2y ago
      I just wanted to point out that, apart from dplyr/magrittr, R introduced a native pipe operator (|>) a few years ago.
  • memhole 2y ago
    For anyone interested in this as design, it’s called method chaining.
    • closed 2y ago
      I think piping and method chaining are a little bit different. Piping generally chains functions, by passing the result of one call into the next (eg result is first argument to the next). Method chaining, like in Python, can't do this via syntax. Methods live on an object. Pipes work on any function, not just an object's methods (which can only chain to other object methods, not any function whose eg first argument can take that object). For example, if you access Polars.DataFrame.style it returns a great_tables.GT object. But in a piping world, we wouldn't have had to add a style property that just calls GT() on the data. With a pipe, people would just be able to pipe their DataFrame to GT().
      • memhole 2y ago
        Good to know. I assumed it was all done via objects or things like objects. So is piping more functional programming?
• satisfice 2y ago
  Interesting how important mere opinion seems to be, because the author doesn't seem to mention two issues that matter a great deal to me: - Alignment of braces and brackets, instead of an opening brace at the end of one line and the closing brace at the beginning of a subsequent line. - everything I need to see is within an eyespan, instead having to jump to several different files to trace code.
• stared 2y ago
  My pet peeve: function getOddness4(n: number): if (n % 2 === 0): return "Even"; return "Odd"; While it is shorter, I prefer vastly prefer this one: function getOddness2(n: number): if (n % 2 === 0): return "Even"; else: return "Odd"; Reason: getOddness4 gives some sense of asymmetry, whereas "Even" and "Odd" are symmetric choices. getOddness2 is in that respect straightforward.
  • CatAtHeart 2y ago
    I personally prefer the former as you can visually see the return one level of indentation below function name. It shows a guaranteed result barring no early-exits. Something about having the return embedded lower just seems off to me.
  • cess11 2y ago
    I would add a blank line to push 'return "Odd";' from the if, and also add brackets around the if-body if the language allows. There are situations where I allow else, they tend to have side effects, but usually I refactor until I get rid of it because it'll come out clearer than it was. Commonly something rather convoluted turns into a sequence of guards where execution can bail ordered based on importance or execution cost. It isolates the actual function/method logic from the exit conditions.
  • __oh_es 2y ago
    I feel guard clauses/early returns end up shifting developer focus on narrowing the function operation, and not an originally happy path with some after thought about other conditions it could handle. IME else’s also end up leading to further nesting and evaluating edge cases or variables beyond the immediate scope of happy path (carrying all that context!).
  • brulard 2y ago
    Nice example of how subjective this is. I immediately thought the first one without "else" is clearly the winner. This is the problem with formatting rules. A codebase needs to have consistent style, even though that might mean nobody is fully happy with it. I for example can not stand semicolons in JavaScript. It is just a visual clutter that is completely redundant, and yet some people really want it there.
• Izkata 2y ago
  I think the only one I disagree with here is the function chains example. I may agree with a different example, but with this one I find the chained version without variables much easier to understand because I'm traversing the graph visually in my head, while the variables are additional state I have to keep track of in my head. ---- Really I was hoping this would be about actual visual patterns and not syntax. It's my major issue with how strict code linting/autoformatting is nowadays. For example, the "black" formatter for python requires this: drawer.ellipse((10, 10, 30, 30), fill=(256, 256, 0)) drawer.ellipse((370, 10, 390, 30), fill=(256, 256, 0)) drawer.arc((20, 0, 380, 180), 15, 165, fill=(256, 256, 0), width=5) The first argument is (x1, y1, x2, y2) of a bounding box and black wants to align x1 for "ellipse" with y1 of "arc". Do people really find that more readable than this? drawer.ellipse( (10, 10, 30, 30), fill=(256, 256, 0) ) drawer.ellipse( (370, 10, 390, 30), fill=(256, 256, 0) ) drawer.arc( (20, 0, 380, 180), 15, 165, fill=(256, 256, 0), width=5 ) Or perhaps something more common in python, kwargs in function arguments. No spacing at all is standard python style: Foobar.objects.create( name=form.name, owner=user, location=form.location, source=form.source, created=time.now, ) Instead for me it's much easier to read this, since I don't have to scan for the "=" and mentally parse each line, it's just a table: Foobar.objects.create( name = form.name, owner = user, location = form.location, source = form.source, created = time.now, ) But no linters/formatters accept such spacing by default. I think flake8 (linter) could be configured to ignore whitespace like this, but black (formatter) can't.
  • jraph 2y ago
    There are two issues with such alignment that may make the "less readable" version a bit better despite being, indeed, arguably less readable: - a modification might lead you to realign several lines, making diffs noisier (though you can ignore white-spaces when displaying diffs, but the commits still hold these lines making navigating in the history less straightforward - we all have our own preferences wrt alignment, and your way of aligning or what you decide to align might be different from someone else, and this can lead to friction Worse is probably better here, as much as I like aligned stuff, I think black is right in this case :-)
  • 2y ago
• gwbas1c 2y ago
  > For long function chains or callbacks that stack up, breaking up the chain into smaller groups and using a well-named variable > Is the second one marginally less efficient? > Yes. No, both versions are just as efficient: In both versions, the same objects are allocated, stored on the heap, and garbage collected. The difference in efficiency comes down to the compiler. For the second version, the compiler should observe that each variable is only used immediately after it's declared, and thus treat those objects as "out-of-scope" as if it's a chained function call.
  • superjan 2y ago
    I agree. After compiling it is even quite likely that the compiler does not care you gave a name to a return value (assuming you let it infer the variable type). What you will see in practice is that the intermediate is explicitly “materialized” (e.g. into a list), because the author wanted to inspect it in the debugger. That will have some cost, mostly in the form of avoidable allocations.
• dchristian 2y ago
  This talks about the "what" of the code, but you have to also convey the "why". If you have a well understood problem space and a team that is up to speed on it, then the "why" is well established and the code is the "what". However, there are often cases where the code is capturing a new area that isn't fully understood. You need to interleave an education of the "why" of the code. I was once asked to clean up for release 10k lines of someone else's robotics kinematics library code. There weren't any comments, readmes, reference programs, or tests. It was just impenetrable, with no starting point, no way to test correctness, and no definition of terms. I talked to the programmer and he was completely proud of what he had done. The variable names were supposed to tell the story. To me it was a 10k piece puzzle with no picture! I punted that project and never worked with that programmer again.
• morning-coffee 2y ago
  > These metrics definitely are debatable (they were made in the 70’s…) What is it about a decade that makes contributions produced thereabout "debatable"?
  • AnimalMuppet 2y ago
    In 50 years, we've learned some things. Not all "good advice for programming" from the 1970s is still actually good advice.
  • russelg 2y ago
    I believe their point is less about the specific decade, but rather they were made over 50 years ago.
• gwbas1c 2y ago
  > To bring closure to the story at the beginning of this post, the codebase that was breaking my brain had several anti-patterns, specifically related to items 1, 2, 3, 6, and 8 in the list above. FYI: If you get into this situation in C#, the .editorconfig file and the "dotnet format" command are a godsend. I inherited a very large, and complicated C# codebase with a lot of "novice" code and inconsistent style. I spent about 3 weeks adding rules to .editorconfig, running "dotnet format" and then manually cleaning up what it couldn't clean up. Finally, I added a Github Action to enforce "dotnet format" in all pull requests. As a result: 1: The code was significantly more readable. 2: It trapped mistakes from everyone, including myself. There are a few areas where we have to disable a rule via #pragma; but these are the exception, not the norm.
  • maleldil 2y ago
    This is a good idea, and I also enforce something similar in my projects. I credit Go for popularising gofmt, including strong defaults and little customisation. My main complaint is that gofmt doesn't break lines.
• slevis 2y ago
  Do people really really agree with "Shorthand constructs that combine statements decreases difficulty"? The author even identifies a problem with the example from the original guide.
  • marcosdumay 2y ago
    No, almost everybody disagree with it as a general statement. Some people disagree to a point where they want languages to have only a handful different constructs. But most people will disagree at some amount of language complexity.
  • Jensson 2y ago
    Everyone agrees that well made shorthand constructs decreases difficulty, since every programmer uses those every day. Things like function calls, while loops etc are all shorthands for different kinds of jump statements combined with register manipulation. Even assembly uses some of those, and I don't think anyone seriously codes in machine code.
  • fauigerzigerk 2y ago
    >The author even identifies a problem with the example from the original guide He does, but I'm not sure he's right. The code snippet appears to be in C# or Dart and neither has undefined.
• stopthe 2y ago
  I always shrugged off the concept of code metrics (from LoCs to coverage) as a distraction from getting actual things done. But since doing more code-review I started to lack a framework to properly explain why a particular piece of code smells. I sympathize with the way the author cautiously approaches any quantitative metrics and talks of them more like heuristics. I agree that both Halstead Complexity and Cognitive Complexity are useless as absolute values. But they can be brought up in a conversation about a potential refactoring for readability. What I didn't find is a mention of a context when reading a particular function. For example, while programming in Scala I was burnt more than once by one particular anti-pattern. Suppose you have a collection of items which have some numerical property and you want a simple sum of that numbers. Think of shopping cart items with VAT tax on them, or portfolio positions each with a PnL number. Scala with monads and type inference makes it easy and subjectively elegant to write e.g. val totalVAT = items.map(_.vat).sum But if `items` were a `Set[]` and some of the items happened to have the same tax on them, you would get a Set of numbers and a wrong sum in the end. You could append to the list of such things until the OutOfMemoryError. But it's such a beautiful and powerful language. Sigh.
  • mrkeen 2y ago
    > But it's such a beautiful and powerful language. Sigh. Don't give up. Half of the time when a good language has problems, it just means that the bad languages don't have those problems yet. You don't need global type-inference and monads to run into your problem. Dynamic languages exist, and even the static ones usually have some kind of 'var x =' local type-inference. And collections like Set probably have a map function.
• 0xbadcafebee 2y ago
  I just wanted to point out that "Cognitive Complexity" [1] was not invented by SonarSource, it is an academic principle created in the 1950s and has more to do with psychology than computer science. Computer science has over-simplified the term to mean "hey there's a lot of stuff to remember this is hard". Psychology tends to have a wider scope of thought and research put into it [2] [3]. For example, one way it's used is not to measure how complex something is, but how capable one particular person is at understanding complex things, versus a different human [4]. This can affect everything from leadership decisions [5] to belief in climate change [6]. I point this out because all too often Engineers hyper-focus on technical details and forget to step back and consider a wider array of factors and impacts - which, ironically, is what cognitive complexity is all about. It's the ability of a person to think about more things in a deeper way. Basically, cognitive complexity is a way to talk about not just things, but people. We also have a tendency as Engineers to try to treat everyone and everything as a blob. We have to design our language in X way, because all people supposedly work in the same way, or think the same way. Or we have to manage our code in a certain way, because all the team members are assumed to work better that way (usually in whatever way is either easier or simpler). One thing I wish people would take away from this, is that not only is cognitive complexity actually useful (it describes how language is able to work at all), but some people are better at it than others. So "avoiding cognitive complexity" is, in many ways, a bad thing. It's like avoiding using language to convey ideas. Language and communication is hard, but you're reading this right now, aren't you? Would you rather a pictogram? [1] https://en.wikipedia.org/wiki/Cognitive_complexity https://en.wikipedia.org/wiki/Cognitive_complexity [2] https://www.jstor.org/stable/2785779 https://www.jstor.org/stable/2785779 [3] https://pubmed.ncbi.nlm.nih.gov/11014712/ https://pubmed.ncbi.nlm.nih.gov/11014712/ [4] https://testing123.education.mn.gov/cs/groups/educ/documents/basic/mdaw/mdaw/~edisp/000089.pdf https://testing123.education.mn.gov/cs/groups/educ/documents... [5] https://deepblue.lib.umich.edu/handle/2027.42/128994 https://deepblue.lib.umich.edu/handle/2027.42/128994 [6] https://www.sciencedirect.com/science/article/abs/pii/S0272494419300192 https://www.sciencedirect.com/science/article/abs/pii/S02724...
• moi2388 2y ago
  I find the main problem is not wanting to split up functions. I greatly prefer small helper functions, so that a more complicated one becomes more readable. Even declaring a little local variable which explains in English what the condition you’re going to test is supposed to do is greatly appreciated
• jedisct1 2y ago
  Rust.
• feoren 2y ago
  > Chaining together map/reduce/filter and other functional programming constructs (lambdas, iterators, comprehensions) may be concise, but long/multiple chains hurt readability This is not at all implied by anything else in the article. This feels like a common "I'm unfamiliar with it so it's bad" gripe that the author just sneaked in. Once you become a little familiar with it, it's usually far easier to both read and write than any of the alternatives. I challenge anyone to come up with a more readable example of this: var authorsOfLongBooks = books .filter(book => book.pageCount > 1000) .map(book => book.author) .distinct() By almost any complexity metric, including his, this code is going to beat the snot out of any other way of doing this. Please, learn just the basics of functional programming. You don't need to be able to explain what a Monad is (I barely can). But you should be familiar enough that you stop randomly badmouthing map and filter like you have some sort of anti-functional-programming Tourette's syndrome.
  • Spivak 2y ago
    authors_of_long_books = set() for book in books: if len(book.pages) > 1000: authors_of_long_books.add(book.author) return authors_of_long_books You are told explicitly at the beginning what the type of the result will be, you see that it's a single pass over books and that we're matching based on page count. There are no intermediate results to think about and no function call overhead. When you read it out loud it's also it's natural, clear, and in the right order— "for each book if the book has more than 1000 pages add it to the set."
    • syklemil 2y ago
      fwiw, once Python's introduced there's the third option on the table, comprehensions, which will also be suggested by linters to avoid lambdas: authors_of_long_books: set[Author] = {book.author for book in books if book.page_count > 1000} These are somewhat contentious as they can get overly complex, but for this case it should be small & clear enough for any Python programmer.
      • davidw 2y ago
• zesterer 2y ago
  I've never understood the hate for variable shadowing. Maybe it's because I mostly use Rust, but I've always found it a useful boon for readability. You often want to extract/parse/wrap/package some value within the middle of a function in a manner that changes its type/form but not its semantic purpose. Shadowing the old value's variable name is brilliant: it communicates that there's a step-change in the responsibilities of the function, demarcating layers from one-another and preventing accidental use of the old value.
  • syklemil 2y ago
    > I've never understood the hate for variable shadowing. Maybe it's because I mostly use Rust, That's likely a good chunk of it. My impression is it's more acceptable in languages where you have a very correctness-focused compiler, and `rustc` is that both with types and liveness/ownership. In a language where it's less clear when you copy values or hand out mutable references, or where implicit conversions occur on type mismatches, it's gonna be a different experience. I think this article is best read as js/ts-specific advice, e.g. the split between null and undefined also isn't something you have to worry about in most other languages, and the semantics of various `?` and `?.` operators can vary a lot.
  • alextingle 2y ago
    If you like to actually read the code, then being able to search for a variable name really helps comprehension. Shadowing makes that harder, by introducing multiple distinct objects with the same name. Now you need something like an IDE to easily follow the lifetime of an object. Introducing a heavyweight dependency like that, as a prerequisite for simply following the code easily, is... a poor choice.
• begueradj 2y ago
  The notion of "readable code" involves 2 parties: a) The skill level of the person who produces the code b) The skill level of the person who reads the code. Most of the time, we tend to blame the a) person.
  • mont_tag 2y ago
    This is true, but it is more nuanced than that. It also has to do with familiarity with common patterns and willingness to accept that not everyone has the same style you do.
• deepsun 2y ago
  > there’s a better chance that the programmer forgets to properly handle all of the possibilities Author totally forgot about IDEs. Yes, I know some coders frown upon IDEs and even color coding (like Rob Pike), but any modern code editor will shout loudly about an unhandled null-pointer check. Also depends on the language, e.g. it's less reliable in Javascript and Python, but in static typed languages it's pretty obvious at no additional cognitive load.
• davidw 2y ago
  This is interesting. Something I long wondered about Lisp code, was how having glyphs that are angled (parenthesis) without much indentation in many cases might be difficult to read just because of the visual aspects of it. ((( (( ( Takes some staring at to figure out what's what.
  • khaledh 2y ago
    This, and the prefix nature of operators. That's the primary reason every time I try to give Lisp a chance, I get turned off by the maze of parens that I have to unravel in my head, especially for long, nested calls. For Lispers, good for them on knowing how to wire their brains to read this effortlessly. For the rest of us, there's a reason why Python's syntax is so easy to read for most people.
    • mtreis86 2y ago
      Yeah it helps if it is properly formatted and intended to be easy to read. Style takes effort. A lot of us let emacs handle the formatting - auto indent and something like paredit to move lists around. Once you get a feel for how the tools move things it is a bit easier to predict, but even then it takes someone putting in effort to make it maximally readable.
  • skydhash 2y ago
    Lisp code is an AST. Once that's internalized, the parenthesis fade in the background. Mentally, instead of editing code, you're just arranging the branches. So, when reading, you can usually ignore whole sections as they will evaluate to a single value (side effects are possible, but strongly discouraged)
    • davidw 2y ago
      Having 4 spaces as indentation helps people tease out where the branches even are in languages like C or Python or whatever, rather than the 1 or 2 that you see with a lot of Lisp. And those angled parents make lining things up vertically a teeny bit more difficult.
• deleted 2y ago
  [deleted]
• seeinglogic 2y ago
  Author here. Thank you for all the thoughtful comments and great stuff I didn't think of (also has been a hot minute since I wrote the article). I appreciate the discussion!
  • superjan 2y ago
    This is, by a large margin, the best article about code complexity I have read in a looong time. Thanks.
• farceSpherule 2y ago
  I was an engineering manager back in the day (Java). People would get lost in the sauce with bracket placement, number of tabs, etc., etc. In order to avoid religious battles, I instituted a code formatter/beautifier. It would format all code upon commit. Problem solved. Although another conversation, people did not want to document their code. So I took the carrot / stick approach. I had to approve all commits and if code did not have javadoc, I did not approve the commit. If your commit was not on time, then that impacted your performance which, in turn, impacted your pay. People bitched at first but whatever. At this particular place, we were trying to get bought. Having documentation and other IP made us more valuable. It forced devs to put actual thought into how to manage their time.
• mwkaufma 2y ago
  All of the leading code samples look _fine_. This whole article is the kind of superficial bikeshedding feedback one gets from code-reviewers who are just phoning it in and don't care about the substance of the problem or solution.
• jorams 2y ago
  This is an interesting article, but also rather unsatisfying. It very quickly jumps to conclusions and goes right back to opinion. I agree with several of those opinions, but opinion was explicitly not the point of the article. > Prefer to not use language-specific operators or syntactic sugars, since additional constructs are a tax on the reader. I don't think this follows from the metric. If a function contains three distinct operators, a language-specific operator that replaces all three of them in one go would reduce the "effort" of function. It's highly scenario-specific. > Chaining together map/reduce/filter and other functional programming constructs (lambdas, iterators, comprehensions) may be concise, but long/multiple chains hurt readability I don't think this follows either. One effect of these constructs when used right is that they replace other operators and reduce the "volume". Again this can go both ways. > ...case in point, these code snippets aren’t actually equivalent! That's a very language-specific diagnosis, and arguably points at hard-to-read language design in JS. The snippet otherwise doesn't look like JS, but I'm not aware of another language for which this would apply. Indeed it is also commonly known as a "null-safe operator", because most languages don't have separate "null" and "undefined". > variable shadowing is terrible > long liveness durations force the reader to do keep more possible variables and variables in their head. These can arguably be contradictory, and that is why I am a huge fan of variable shadowing in some contexts: By shadowing a variable you remove the previous instance from scope, rather than keeping both available.
• indulona 2y ago
  [dead]
• ezoe 2y ago
  Any nesting and indirection burden the mental fatigue on understanding the code: nesting conditional statements, macros, functions, classes while it's necessary to use or we will have so many duplicated codes which also burden the mental fatigue anyway. There was an extreme argument on a SNS recently that someone claimed that he prohibit nesting if in their work. Shorter-lived Variables argument doesn't always work. One of the most horrible code I read use very short-lived variables: val_2 = f(val), val_3 = g(val), ... It's Erlang. Because Erlang's apparent variable isn't a variable, but just a name bound to a term.
• TrianguloY 2y ago
  As someone with relatively bad memory, in general less variables = easier to understand. The more I need to remember, the more time I'll spent. IDEs help by providing inline hints and documentation, but the main issue is that a variable _can_ be used later, so you need to remember it. No variable (like on a chained/piped construct) means that the value is generated and immediately used.
• m463 2y ago
  I enjoy the visual uncomplexification of python indenting. After switching back and forth with languages with "extra" syntax, it seems visually and cognitively cleaner. that said, there were some things about perl that I liked cognitivel, like being able to say "unless" instead of "if not"
• odyssey7 2y ago
  When are they planning on updating the simple sabotage field manual pdf?
• Duanemclemore 2y ago
  It would be interesting to see how APL and other array languages stack up in this kind of analysis. I'm guessing they would do well...
• throwaway2037 2y ago
  This image shows six different ways to write a simple function, getOddness(): https://seeinglogic.com/posts/visual-readability-patterns/6-oddness.png https://seeinglogic.com/posts/visual-readability-patterns/6-... Personally, my normal style is getOddness2(). I try to never have an expression in my return statement -- only return a literal, local variable, or class data member. Why do I choose getOddness2()? It is so easy to debug. When I write code, I am mostly thinking about difficult to debug -- control flow and local variables. I would like to hear about other people's style and why they choose it. Related: Does Google Code style guidelines (perhaps the most famous of such guidelines on the Interwebs) have anything to say about which version of getOddness() is best/recommended?
• userbinator 2y ago
  Smaller functions with fewer variables are generally easier to read I hate how a lot of focus on "readability" is on micro-readability, which then tends to encourage highly fragmented code under the highly misguided assumption that micro-readability is more important than macro-readability. The dogma-culting around this then breeds plenty of programmers who can't see the forest for the trees and end up creating grossly inefficient code and/or have difficulty with debugging. APL-family languages are at the other extreme, although I suspect the actual optimum is somewhere in the middle and highly dependent on the individual.
  • DimmieMan 2y ago
    There's certainly a middle ground, especially when multiple file are involved. 3-4 go to definitions in something i'm not familiar with and i'm struggling, now that's a me problem but i can't imagine most people are miles ahead of me. .Net culture, especially with "clean architecture" is shocking for this, you go to modify a feature or troubleshoot and things are spread across 4 layers and 15 files, some that are > 60% keywords. I don’t have an answer of where the cutoff is but I'll generally take 1 longer function that's otherwise neat and following the other recommendations outlined that I can read sequentially instead of scrolling up and down every 5 lines because it's so fragmented. same can be said for types/classes too, that 4 value enum used only for this DTO does not need to be in another file!
    • neonsunset 2y ago
      What’s tragic is this is completely self-inflicted and you could argue is done against the kind of code structure that is more idiomatic to C#. Luckily, you don’t have to do it if you are not already working with this type of codebase.
• hackburg 2y ago
  [dead]
• zombot 2y ago
  > Let me be upfront: there is no commonly-used and accepted metric for code readability. Indeed, I haven't found an autoformatter yet whose default settings I find "readable". The best of them is still clang-format, because it has so many parameters I can tune. By now I have a .clang-format config file that almost comes close to producing the formatting I do manually. I also find functional patterns way more readable than their procedural counterparts.
• austin-cheney 2y ago
  I have seen this subject come up as the most important factor of human behavior in every one of my employments. Counting both my corporate time and military time that is nearly 50 years employment time at more than 20 different organizations. First things first, complex is a fancy word that means many. That's it, so don't over think it. Everything else is bias and opinions. If you feel conflicted about that see the fantastic Rich Hickey talk: Simple Made Easy https://www.youtube.com/watch?v=SxdOUGdseq4 https://www.youtube.com/watch?v=SxdOUGdseq4 Secondly, how this actually manifests is that people are eager to solve some problem in a way they feel comfortable, whether by process or automation, and then nobody wants to maintain anything, especially if it was written by somebody else. This is a critical failure. The best solution I have found to solve for this is speed, not code style or anything else. Uncertainty increases proportionally to the duration between testable iterations. For example if it takes 30 seconds to try out an idea and reset back to the prior state people feel more certain about the environment in which they work because risk is low and learning is fast. If, on the other hand, it takes 90 minutes to try out an idea people will be exceedingly cautious about what they try and propose because everything is fragile and expensive. Knowing that speed is the solution is not convincing in a world where most software developers cannot measure anything. Instead it must be mandated that software has only one purpose: automation, so automate the shit out of it. When that becomes the thesis of all work speed naturally self-amplifies and people split into two camps. The first camp is people that can actually write original software and the second camp is people who will kill you with internal processes, such as complexity checks and code style rules, because they cannot automate their way out of it.
• alan_n 2y ago
  I think a lot about code readability a lot. I agree with most of this except the names part and shorthand constructs. Especially for the latter, it's unfair to make a comparison with the wrong operator. The example in general is a bit weird. I think a better one is `if (myObj !== null) myObj = someOtherObj`. A very common pattern for which js now has a nullish coalescing assignment you can use insteal (`myObject ??= somOtherObj`) or there's also ||=. These sacrifice some readability upfront due to them being "uncommon" operators and the code might be harder to read for someone whose never seen it, but is so much easier to read once you get used to it. There is of course, always a trade off. Using a lot of .? to access properties can be code smell, but also sometimes very helpful, as the alternative would expand into a lot of code. Regarding names, I think the suggestions are a bit in conflict, as often to avoid variable shadowing you have to do stuff like name things node, _node, node2. I try to have distinct names, but I'd rather the shadowing in those cases where it's hard. As for i, and j. I don't like them, but they're such conventions it's hard to avoid them. I always try to use them only once, and assign the variable I need: `item = obj[i][j]` if possible.