12 ms·

Please do not attempt to simplify this code

479 points by whalesalad 2y ago

jftuga 2y ago
// ================================================================== // PLEASE DO NOT ATTEMPT TO SIMPLIFY THIS CODE. // KEEP THE SPACE SHUTTLE FLYING. // ================================================================== // // This controller is intentionally written in a very verbose style. You will // notice: // // 1. Every 'if' statement has a matching 'else' (exception: simple error // checks for a client API call) // 2. Things that may seem obvious are commented explicitly // // We call this style 'space shuttle style'. Space shuttle style is meant to // ensure that every branch and condition is considered and accounted for - // the same way code is written at NASA for applications like the space // shuttle.
- wouldbecouldbe 2y ago
  hahaha well Kubernetes is the opposite of a special shuttle that keeps on flying. It crashes all the time, version updates etc. If you want stability go to apache or nginx.
  - Sohcahtoa82 2y ago
    This comment is a complete non-sequitor. Kubernetes solves an entirely different problem from Apache and nginx.
    - deleted 2y ago
      [deleted]
    - wouldbecouldbe 2y ago
      yeah maybe it does, but when you have a hammer everything looks like a nail. Kubernetes is used all the time nowadays on simple projects just because it's the most intellectual pleasing solution. This is creating high maintainance costs by locking in companies with extremely complicated software where it's just not needed.
  - andix 2y ago
    I've never seen Kubernetes crash. I don't have that much experience with operating k8s clusters, but on those I've seen it just kept on working.
    - whalesalad 2y ago
      Same. Pods will crash, run out of memory, fail to get scheduled, etc... but I have never seen kube itself crash.
      - q3k 2y ago
        I've been experimenting with k8s and/or running it on prod basically since the initial release, and I've so far only had one workload-affecting bug. The bug caused the scheduler to get stuck meaning pods assigned to nodes kept running, but no new pods would be scheduled. https://github.com/kubernetes/kubernetes/issues/124930 https://github.com/kubernetes/kubernetes/issues/124930 That's a pretty good track record, and indicates a level of fail-safe design (everything continued working, even though a critical components kept crashinglooping).
    - kesor 2y ago
      You obviously have not operated kubernetes long enough. With large enough scale you'll find the cluster controllers crashing for various reasons, getting out of sync with each other, etcd crashing or getting locked, and a whole bag of bugs.
  - OJFord 2y ago
    If you can solve your problem with nginx then yes you should be using nginx not kubernetes...
  - nimih 2y ago
    If you think nginx is the right tool for solving problems like container deployment, service discovery, cluster scaling, and secret management, then I suppose it's not surprising that you think Kubernetes "crashes all the time" and that a 1 year rolling support window for software releases is an insurmountable obstacle. Kubernetes has a lot of genuine issues and rough edges, but you're kind of showing your ass when you make comments like this.
    - wouldbecouldbe 2y ago
      There are just very few applications that actually need all of this. maybe 1 tot 0.1%, for instance vercel might need it. But 95-99% can just run on several "simple" servers & keep deployment times within minutes and no complicated stuff needed. Yet Kubernetes get's pushed all the time.
      - lyu07282 2y ago
        I don't know about that, I've found even simple applications need things like blue/green deployments and kubernetes makes that very easy and robust.
        wouldbecouldbe 2y ago
        there is nothing easy or robust about kubernetes. Hence all the tooling around it, having things break down if you don't update. Dependencies not being compatible all the time. Server management should cost as little time, and be stress free. There are many ways to solve the up/down issue and depends on which language you are running.
        lyu07282 2y ago
        I agree if you are running kubernetes yourself you are absolutely right. I was thinking more about managed clusters in the cloud. Every provider offers managed kubernetes, then you aren't even vendor locked.
- whalesalad 2y ago
  I tried to link directly to these lines, but hn's url sanitizer stripped off the anchor tags.
- arwhatever 2y ago
  When I looked into Go I found it a bit surprising that someone had created a non-expression-based language as late as ~2009. I have not familiarized myself with the arguments against expression-based design but as a naive individual contributor/end-user-of-languages, expressions seem like one of the few software engineering decisions that doesn't actually "depend," but rather, designing languages around expressions seems to be unequivocally superior.
  - TurningCanadian 2y ago
    instead of "expression" you meant "exception", right?
    - whalesalad 2y ago
      expression as-in s-expression (ex: lisp)
    - Jtsummers 2y ago
      In the statement/expression-oriented axis of languages, Go is a statement oriented language (like C, Pascal, Ada, lots of others). This is in contrast to expression oriented languages like the Lisp family, most, if not all, functional languages, Ruby, Smalltalk and some others. Expressions produce a value, statements do not. That's the key distinction. In C, if statements do not produce a value. In Lisp, if expressions do. This changes where the expression/statement is able to be used and, consequently, how you might construct programs.
      - runevault 2y ago
        A simple example for anyone who might not appreciate why this can be so nice. In languages where if is a statement (aka returns no value), you'd write code like int value; if(condition) { value = 5; } else { value = 10; } Instead of just int value = if(condition) {5} else {10} Some languages leave ifs as statements but add trinary as a way to get the same effect which is an acceptable workaround, but at least for me there are times I appreciate an if statement because it stands out more making it obvious what I'm doing.
- supportengineer 2y ago
  I went right into the code and looked for 'if' statements without 'else' statements. There are plenty. I don't see how you can have any exceptions to this rule if you are truly committed to capturing all branches.
  - throwanem 2y ago
    If the 'if' condition matching always results in a thrown exception, a return, or likewise, then you don't really need an 'else' unless you're using a language which supports conditions and resumption (conformant Common Lisp implementations, and not really anything else I know of). The 'else', implicitly, is that the flow of control leaves the scope of the 'if' block at all. (I haven't read far enough into the code to know that this is what they're doing, but the head matter I did read suggests as much. It's a common enough pattern, especially around eg argument validation and other sanity checks a function might perform to ensure it can do meaningful work at all.) (I do wish HN supported an inline monospace markup, the <code> to a four-space indent's <pre>...)
    - iainmerrick 2y ago
      Swift's "guard" statement would be pretty handy here.
      - drewcoo 2y ago
        if (foo) { bail out } They're known as guard statements regardless of language. https://en.wikipedia.org/wiki/Guard_(computer_science) https://en.wikipedia.org/wiki/Guard_(computer_science) Swift has a guard keyword, but the construct feels a little awkward given most languages do the above. It makes me do a double take. guard !foo else { bail out }
        iainmerrick 2y ago
        No, "guard" in Swift is special. Once you're in the "else" block, you MUST return or call a non-returning function (e.g. abort). It's specifically designed to prevent bugs where flow control accidentally resumes from an error handler. It's the same idea as the "every 'if' must have an 'else'" guideline in the code being discussed, except with "guard" the compiler will detect violations. It's a good thing.
- deleted 2y ago
  [deleted]
jerlam 2y ago
// KEEP THE SPACE SHUTTLE FLYING. I understand the intent, but it is a bit funny that the comment references a system that is no longer operational due to its poor safety record. In ten years or so, will people even remember the Space Shuttle in a good light?
- whalesalad 2y ago
  Well over 100 successful missions carrying a bunch of people and gear up into outer space and then bringing them back home. I hold it in a good light now, and will likely continue to feel that way. As far as human progress and net good, it was a success.
  - joebob42 2y ago
    100 is frankly not that impressive for software. A 2-9 system will sometimes work 100x in a row, and a 3-9 system usually will.
    - rco8786 2y ago
      100 missions. Not 100 code executions.
      - joebob42 2y ago
        Sure, but I imagine at least some components only really execute a small number of times per flight, or possibly never in the case of certain error handling code. Stretching the metaphor more than is probably appropriate, I'd treat launching the shuttle and having it come back as a big integration test. A system that passes it's integration test 100 times isn't necessarily particularly impressive in terms of reliability. We run our integration test tens of times a day, and it fails once or twice a month. Our system is kinda flaky :(
        rco8786 2y ago
        > A system that passes it's integration test 100 times isn't necessarily particularly impressive in terms of reliability. So extending your own metaphor and using 100 as the number of missions, the integration test failed 2% of the time.
vednig 2y ago
Main reason it is, as it is, and is usually not seen in any other languages is that go does not has operator functions for variable fallbacks and logical error handling. But overall this helps in improving performance of application(on a large scale).
boilerupnc 2y ago
Related article on Space Shuttle Software Quality [0] Excerpt: "But how much work the software does is not what makes it remarkable. What makes it remarkable is how well the software works. This software never crashes. It never needs to be re-booted. This software is bug-free. It is perfect, as perfect as human beings have achieved. Consider these stats : the last three versions of the program — each 420,000 lines long-had just one error each. The last 11 versions of this software had a total of 17 errors. Commercial programs of equivalent complexity would have 5,000 errors." [0] https://archive.is/HX7n4 https://archive.is/HX7n4
- deleted 2y ago
  [deleted]
- emerongi 2y ago
  5000 / 17 ≈ 295. Is it a fair assumption to make that a commercial program of equivalent complexity would take 295x fewer man-hours?
  - fleischhauf 2y ago
    without any knowledge of the other processes involved in space shuttle software development, I highly doubt that the correlation is that easy
  - burkaman 2y ago
    No, I don't think so, 295x is a crazy high factor. The article says 260 people are involved, and let's generously say it took 20 years to write the software (the first mission was 10 years after the program started and it was around for a total of 40 years). Dividing by 295 means a commercial team of the same size could have done it in less than a month. Or with a 10x smaller team, about 8 months. I don't think either of those are plausible.
    - deleted 2y ago
      [deleted]
    - swiftcoder 2y ago
philip1209 2y ago
Ah, 2k-line files makes me miss my days of coding in Go.
- pizzafeelsright 2y ago
  Join me. Still crushing it.
- LAC-Tech 2y ago
  2k lines is a whole widely used and feature filled open source library in many languages.
  - stitched2gethr 2y ago
    Yes, but often density is at odds with readability.
    - LAC-Tech 2y ago
      As someone who has coded in languages of different density, I disagree. It's very easy to lose context and the big picture when you're scrolling around code written in a non dense language. (I'm talking the difference between say... Java and F#. I can't say much for the more extreme differences like COBOL and APL)
- theshrike79 2y ago
  It's a lot easier to glance through a single 2k line file than it would be to go through 200 separate 100 line files (+ extra overhead for imports in each file)
jsbg 2y ago
> // 1. Every 'if' statement has a matching 'else' (exception: simple error > // checks for a client API call) > // 2. Things that may seem obvious are commented explicitly Honest question: Why invent "safety" practices and ignore every documented software engineering best practice? 2,000 line long modules and 200-line methods with 3-4 if-levels are considered harmful. Comments that say what the code does instead of specifying why are similarly not useful and likely to go out of date with the actual code. Gratuitous use of `nil`. These are just surface-level observations without getting into coupling, SRP, etc.
- anonymoushn 2y ago
  Is there any evidence that these things are harmful or just vibes?
- bayindirh 2y ago
  Because sometimes, there's "No Other Way(TM)". Arbitrary line limits tend to unnecessary fragmentation. Add includes, licenses, glue code and comment; and you have an unapproachable spaghetti. Try to keep methods to 200 lines in high performance code, and see your performance crash and burn like Icarus' flight. When you read the comments in the code, you can see that they simplified the code to a single module, and embedded enormous amount of know-how to keep the code approachable and more importantly, sustainable. For someone who doesn't know the language or the logic in a piece of code, the set of comments which outline what the code does is very helpful. In six months, your code will be foreign to you, so it's useful for you, too. Comments are part of the code and the codebase. If you're not updating them as you update the code around them, you're introducing documentation bugs into your code. Just because the compiler doesn't act on them doesn't mean they are not functional parts of your code. In essence they're your knowledge, and lab notebook embedded in your code, and it's way more valuable in maintaining the code you wrote. They are more valuable than the code which is executed by the computer. Best practices are guidelines, not laws or strict rules. You apply them as they fit to your codebase. Do not obey them blindly and create problematic codebases. Sometimes you have to bend the rules and make your own, and it's totally acceptable when you know what you're doing.
ChrisArchitect 2y ago
Discussion from 2018: https://news.ycombinator.com/item?id=18772873 https://news.ycombinator.com/item?id=18772873
- whalesalad 2y ago
  wtf I submitted this in 2018?????
  - pranshum 2y ago
    Our obsessions never change!
    - whalesalad 2y ago
      Nothing is quite as exciting as exploring the implementation of persistent volume claims
      - xarope 2y ago
        There's a Borges story lurking here...
  - daedrdev 2y ago
    The joys of the human memory
  - scj 2y ago
    That begs the question: What brought you to this file?
    - whalesalad 2y ago
      I saw it in the kube reddit, got a chuckle, and figured I would share here. Guess I got a chuckle 6 years ago too! Can't remember how I found it last time.
  - deleted 2y ago
    [deleted]
EgoIsMyFriend 2y ago
Previous discussion https://news.ycombinator.com/item?id=18772873 https://news.ycombinator.com/item?id=18772873 (Dec 2018, 1552 upvote, 631 comments)
- dangsux 2y ago
  By the same user no less
renewiltord 2y ago
Why was Space Shuttle code so good and the engineering so bad? The thing was expensive and shit and had a 1.5% catastrophic failure rate for passenger transport. Soyuz was two orders of magnitude better. Russia/USSR have reputation for McGyvering things and US has reputation for gold-plating but US ship is killing people every 65 flights and Russian ship has over 1500 launches without death. Maybe engineers should learn from either Soviet/Russian engineers or from NASA software engineers. Both are making more reliable things.
- teractiveodular 2y ago
  Multiple books have been written on this very topic, but the TL;DR is that the problem was not the engineering, but the absurd, often mutually contradictory design decisions forced on it for political reasons. https://en.wikipedia.org/wiki/Space_Shuttle_design_process https://en.wikipedia.org/wiki/Space_Shuttle_design_process
  - nvarsj 2y ago
    The irony is this kind of describes k8s as well.
    - Yasuraka 2y ago
      How so?
      - nvarsj 2y ago
        Hmm I guess my comment was kind of tongue in cheek. I think a lot of k8s was driven by political or pet-project reasons. E.g. using etcd.
- ranger207 2y ago
  https://waynehale.wordpress.com/2012/11/12/after-ten-years-a-few-words-from-admiral-gehman/ https://waynehale.wordpress.com/2012/11/12/after-ten-years-a... > I need to give you the issue from the NASA point of view so you can understand the pressures that they were under. In a developmental program, any developmental program, the program manager essentially has four areas to trade. The first one is money. Obviously, he can go get more money if he falls behind schedule. If he runs into technical difficulties or something goes wrong, he can go ask for more money. The second one is quantity. The third one is performance margin. If you are in trouble with your program, and it isn’t working, you shave the performance. You shave the safety margin. You shave the margins. The fourth one is time. If you are out of money, and you’re running into technical problems, or you need more time to solve a margin problem, you spread the program out, take more time. These are the four things that a program manager has. If you are a program manager for the shuttle, the option of quantity is eliminated. There are only four shuttles. You’re not going to buy any more. What you got is what you got. If money is being held constant, which it is—they’re on a fixed budget, and I’ll get into that later—then if you run into some kind of problem with your program, you can only trade time and margin. If somebody is making you stick to a rigid time schedule, then you’ve only got one thing left, and that’s margin. By margin, I mean either redundancy—making something 1.5 times stronger than it needs to be instead of 1.7 times stronger than it needs to be—or testing it twice instead of five times. That’s what I mean by margin. > It has always been amazing to me how many members of Congress, officials in the Department of Defense, and program managers in our services forget this little rubric. Any one of them will enforce for one reason or another rigid standard against one or two of those parameters. They’ll either give somebody a fixed budget, or they’ll give somebody a fixed time, and they forget that when they do that, it’s like pushing on a balloon. You push in one place, and it pushes out the other place, and it’s amazing how many smart people forget that.
neilv 2y ago
BTW, when linking to a file in GitHub like this, you can link to a range of lines of the file, by using the URL fragment identifier, like: #L60-L92 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb454ce69dee737e3eb91a25e0535/pkg/controller/volume/persistentvolume/pv_controller.go#L60-L92 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb4...
- runlevel1 2y ago
  You can click a line number to select the start of the range, then shift+click to select the end of the range to do this automatically.
- ivanjermakov 2y ago
  I think the whole file worth skimming through
- whalesalad 2y ago
  Try submitting a URL to HN that has an anchor tag and see what happens =)
- corytheboyd 2y ago
  I (well really, a coworker of mine) just today discovered a JetBrains action “Copy GitHub URL” [sic] that, if you have lines of code selected in the IDE, includes those lines in the copied URL fragment. So so so much better than my old workflow of stopping what I’m doing, going to the file in GitHub, and selecting the lines there to share links to bits of code.
  - croemer 2y ago
    Same exists in VScode, probably need some GitHub extension for it.
kylehotchkiss 2y ago
Is this like a thing where people want to appear to be contributing to open source and remove all the 'unneeded' elses from a codebase?
- dangsux 2y ago
  [flagged]
quotemstr 2y ago
This sort of code strikes me as an ideal candidate for translation into a declarative, rule-based, table-driven system. Such a thing is more comprehensible and more verifiable than ad-hoc imperative if-clause-rich code. Messy code of this sort is usually a sign of a missing abstraction.
- School-Cotton 2y ago
  The Go ideology is basically to just write down all the code, in a more or less straightforward translation of what you would have written in C, and not to try to abstract anything.
  - GuB-42 2y ago
    Which is an underrated ideology. I once had to go through some code that was objectively terrible. The guy who wrote was is a mechanical engineer, close to retirement at the time, and self-learned in programming. He had absolutely none of the background you can expect a professional programmer to have, and in particular abstraction seemed like a foreign concept to him. Have 50 buttons, each doing essentially the same thing, and you will see 50 copy-pasted blocks of code. Particularly ironic that he was using Java, a language known for its culture of abuse of design patterns and abstraction. But despite that huge mess that code was, it was surprisingly readable. You could look anywhere in the code and understand what it is doing. No calling though interfaces, when you see foo.bar(), you can just follow the symbol in your IDE and that's the instruction that will be run, and many times, there is not even a function, just code, thousands of lines of it, different cases are just dealt with ifs. Maybe it was the most pleasant "bad code" I had to work with. Code using the wrong or too much abstraction is much worse, because it is just as buggy and ugly, but you don't even know what to expect when all you see is an interface call where the actual code is in a completely different part of the software, and what connect the two is in yet another place. With more "factories", "managers", "dispatchers", etc... than actual logic.
    - globular-toast 2y ago
      If you saw the 50 copy-pasted blocks of code I'll bet you mentally abstracted it and assumed they were all doing the same thing. The problem comes when one of them isn't quite doing the same thing. That isn't possible with a real abstraction. Perhaps as a mechanical engineer the guy understood the more important things, though, like separation of concerns and a layered model, ie. architecture. Truly bad code mixes up all concerns into one ball of mud. Feel like forking a new process right from the GUI layer (the only layer) based on some business logic for that one button press? No problem! Being able to look at a single piece of code in isolation and understand what it's doing isn't the challenge. Anyone can do that for any code. The challenge is knowing how it runs in context of the larger program. What are the downstream implications of the way it's done? How many times is this run and why? Who is this code responsible to and why would it change (e.g. is it business logic, or just a UI thing)? This is the kind of thing an architecture gives you. You don't need to go all in with abstracting everything, but you do need some architecture.
copypasterepeat 2y ago
I've obviously only skimmed the code, but honestly it doesn't look that bad to me. Sure there are things I would do differently, but I've seen much, much worse. At least the code follows a single convention, and has the appearance that everything was thought through and that there is method behind the madness, as it were. I'd take this any day over the typical mishmash of styles, lazy coding, illogical code structure etc. that I've encountered so many times.
matthewmacleod 2y ago
Personally I love this level of verbosity in code. There are still way too many levels of nested control flow for my taste—I find that makes it exceptionally hard to retain context—but at least there are early returns.
- theshrike79 2y ago
  With a half decent editor you can fold them down so that just the comment on top of the if branch is visible
dpedu 2y ago
I wrote a toy kubernetes CSI driver recently and found it quite pleasing to do. Bare minimum means implementing just 3 grpc api calls - An informational one and publish/unpublish. Amazon's EFS or EBS CSI drivers are good examples because they're a pretty small codebase. I don't know exactly how this code interacts with the CSI driver itself, but it appears that it is the logic that ultimately results in the volume manipulation calls the controller makes against the CSI driver. It's nice that the complexity is all here, I was actually pretty surprised how simple the drivers themselves are.
wavemode 2y ago
Am I weird in feeling like the code in this file is really really... normal? Like, it's verbose in certain ways due to being written in Go, as well as due to not relying on any deep abstractions (and I don't mind this - abstractions are a double-edged sword), but in general, as code, it seems typical - and if the header text didn't exist I wouldn't think twice about the style it's written in. Maybe the disconnect here is that most of my experience is in enterprise software rather than systems software. Perhaps many of the comments in this file seem unnecessary to regular contributors within the k8s project? Whereas if I were writing this same code in an enterprise (and thus expect it to be read by people far in the future lacking context on all the technical details) I would have put -more- comments in this file, given the sheer complexity of all it's doing.
- mumblemumble 2y ago
  I wish code like this still felt normal to me, but over the past ~10 years it seems that many people have come to value brevity over explicitness. I strongly prefer the explicitness, at least for important code like this. More than once in my career I've encountered situations where I couldn't figure out if the current behavior of a piece of code was intentional or accidental because it involved logic that did things like consolidating different conditions and omitting comments explaining their business context and meaning. That's a somewhat dangerous practice IMO because it creates code that's resistant to change. Or at least resistant to being changed by anyone who isn't its author, though for most practical purposes that's a distinction without a difference. Unnecessarily creating Chesterton's fences is anti-maintainability.
  - bhauer 2y ago
    Agreed. Explicitness and comments are very useful in understanding the intended functionality and logic, whether or not the code actually implements that intent correctly (an in providing that intent, they can help identify bugs earlier than they would be identified otherwise).
    - dskrvk 2y ago
BaculumMeumEst 2y ago
I do like the thought of firing Kubernetes into space.
sam_perez 2y ago
Beautiful. ---------------------------- This controller is intentionally written in a very verbose style. You will notice: 1. Every 'if' statement has a matching 'else' (exception: simple error checks for a client API call) 2. Things that may seem obvious are commented explicitly We call this style 'space shuttle style'. Space shuttle style is meant to ensure that every branch and condition is considered and accounted for - the same way code is written at NASA for applications like the space shuttle. ---------------------------- ^^^^ This bit reminds me of exhaustive checks in typescript code. I try to use them all the time. https://www.typescriptlang.org/docs/handbook/2/narrowing.html#exhaustiveness-checking https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...
- shepherdjerred 2y ago
  You might like ts-pattern https://github.com/gvergnaud/ts-pattern https://github.com/gvergnaud/ts-pattern
- bubblyworld 2y ago
  Hah, years of writing ts for various contracts and I had no idea that never could be used like this. Thanks!
- jeremyjacob 2y ago
  The newer `satisfies never` is great for this purpose. It’s also convenient with if else chains if that’s more one’s style.
bsder 2y ago
When, oh, when, will we get a programming language that actually supports state machines properly ...
- tashmahalic 2y ago
  What does supporting state machines properly look like?
  - bsder 2y ago
    Dunno. Maybe a table with functions and conditions? The point isn't the specific form that support would take. The point is that it has some form that implies the intent of the programmer. It has N states and they are enumerated. It has code that runs on transition and state. It has these variables which make up conditions. The state doesn't become a dead end and all states are reachable. etc. The point is to enable the compiler to know "Hey, this is a state machine. You can check <invariant> because it is a state machine.
zoogeny 2y ago
// CSINameTranslator can get the CSI Driver name based on the in-tree plugin name type CSINameTranslator interface { GetCSINameFromInTreeName(pluginName string) (string, error) } Do people actually find comments like the above useful?
- deleted 2y ago
  [deleted]
- jakevoytko 2y ago
  On a big open-source project like Kubernetes, they're probably happy with the tradeoff between "some exported names have inane and obvious comments" and "our linter requires open-source contributors to document their exported names in a consistent way."
- sollniss 2y ago
  With the official linter (golint), exported types must be commented, so sometimes you get Captain Obvious comments like this.
- kmoser 2y ago
  Yes, if it's your first day on the job and you don't know that CSI is a driver. Yes, if GetCSINameFromInTreeName ever gets renamed to something less obvious.
- rk06 2y ago
  yep, they are useful. I consider comments to live at "conceptual" level while code lives at "physical" level. With this way,when you are debugging, your mind can read code at conceptual level and easily disregard irrelevant blocks of code. without comments, i will need to mentally drop down at physical level and implement a compiler in my brain for the same results
- akvadrako 2y ago
  In this example it's faster for me to understand what the code does by reading the comment than to parse the code itself. If you're just scanning over a lot of code and looking for what you care about it could be helpful.
inamberclad 2y ago
If people want to look at some Real Deal Space Code, here's some parts I like: CoreFlight System's executive code: https://github.com/nasa/cFE/tree/main/modules/es/fsw/src https://github.com/nasa/cFE/tree/main/modules/es/fsw/src Any code from the core of RTEMS: https://gitlab.rtems.org/rtems/rtos/rtems/-/tree/main/cpukit/score?ref_type=heads https://gitlab.rtems.org/rtems/rtos/rtems/-/tree/main/cpukit... The code isn't special but it is neat and tidy, and very very clear. It says what it does, and it does something small and does it well.
CGamesPlay 2y ago
Is this code unit tested? If it's so critical that every branch has to be accounted for, I would assume that time would be well-spent combinatorically testing its inputs, right?
verandaguy 2y ago
Speaking specifically about cases where any not-completely-trivial `if` is matched with an explicit `else`: I wonder to what extent this code could be simplified if the authors of k8s had chosen to design around using structural pattern matching rather than `if`/`else` blocks? Lots of mainstream languages with support for structural pattern matching have compile-time tooling to check whether a match was exhaustive, which alone could serve as an idiomatic solution while increasing information density in the code.
deleted 2y ago
[deleted]
readthenotes1 2y ago
One part of what NASA did was have every line of code reviewed by many people, including someone whose sole job was to make sure the comments matched the code and vice versa. If you do not take such care, excessively for both comments are inevitably going to drift from the code and lead to incredible confusion.
geuis 2y ago
Joined a new company recently. They kept telling me their codebase was a mess. Yet I've been finding it to be remarkably refreshing. There are extensive comments everywhere. Lots of white space (remember to let your code breathe). Existing linting policies are extensive and thorough. The code is well structured and remarkably easy to follow through where that does what. I think it helps that it's a very small team with maybe 5-6 people max over time. It's such a pleasure to explore the project while learning the system and starting to fix some small long term issues. Really, really nice.
- whalesalad 2y ago
  This is a great sign because it shows humility of the engineering team.
wvenable 2y ago
I would consider this a code smell: if err != nil { return "N/A" } And I overall dislike the level of nesting in some of these functions but that might just be the nature of Go code.
nullc 2y ago
But do they have tests that achieve 100% condition/decision branch coverage? If they do, changes that disrupt handling some cases ought to be detected by the tests.
- globular-toast 2y ago
  I assume no and ultimately that is the point of code like this. As you say, if you can somehow correctly identify every possible branch in your tests then you could write the actual code any way you like. But then the tests would have to look like this, otherwise you'd have abstraction in your tests and you couldn't be sure if it covers every branch. Who tests the tests?
  - nullc 2y ago
    You test the test by mutating the code. Once you have 100% condition/decision branch coverage you can automatically sweep the code with changes and the tests will fail. I have a little harness I use for this steps through each non-comment line of code changes signs, comparison directions, offsets values, swaps variables, adds negations, replaces computations with constants, etc. basically changes that are more or less guaranteed to compile. Then it runs the compiler to produce a stripped optimized output, if the compiler is successful, it checks that the resulting md5 is different from all the prior results, runs the tests. If the tests pass, it saves the passing code (which, to be clear is a meta-test failure), and then later I sweep through and either determine that it managed to produce functionally equivalent code or I improve the tests (and fix the resulting bugs they expose). The identical compiled binary test eliminates a lot of false positives. But that kind of approach doesn't work unless you get to ~100% condition/decision branch coverage since obviously any condition that isn't tested will be free to mutate. Hm. Maybe if I updated it I'll have some attempt at sampling LLM rewrites of functions. :P
    - globular-toast 2y ago
      Interesting approach. I hadn't considered actually implementing such "brute force" methods. I guess it's similar to fuzzing. I think the problem is you are then moving your "real" condition/decision branch documentation into your tests. The tests are then basically a guard rail for just in case someone modifies some abstract bit of code and it changes the behaviour of some otherwise opaque decision branch. The approach in OP seems to be to just move the "real" logic/documentation into the code itself and do away with the abstractions (and perhaps the tests too).
jmmv 2y ago
Nice. I wrote about a similar idea back in 2013 (https://jmmv.dev/2013/07/readability-explicitly-state.html https://jmmv.dev/2013/07/readability-explicitly-state.html) after I found that being explicit about all branches made my code easier to reason about and easier for reviewers to validate. Glad to find that this is “space shuttle style”! I’ve always disliked how the Go style insists on removing certain branches at the end of functions, for example.
raffraffraff 2y ago
It's an amazing code style for training LLMs.
edpichler 2y ago
When we have this on code is because we failed in something else.
serserser 2y ago
when i see too much verbosity i sometimes think someone is trying to hide something behind it...
dustedcodes 2y ago
I love this style of coding and it's something I've been doing increasingly more over the last few years. Code verbosity is so underrated in so many dev teams. I have never had a co-worker complain that my code was too well documented and too easy to understand, quite the contrary actually. When someone advocates for more concise code by saying that it's "easier and quicker to read" I always counter that if you use a lot of language feature magic to make code more concise then the mental work to read the code remains the same, the only difference is that you ask your co-workers to leave the editor and google lots of things in order to understand your brief code versus keeping them in their editor and just being able to read the simple verbose code without interruption in one place.
otikik 2y ago
This kind of thing should be enforced with linting rules and an automated action that rejects any pull request which violates the rules, not with a comment.
camgunz 2y ago
Can someone please get gofmt to wrap lines. This [0] is ridiculous. [0]: https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb454ce69dee737e3eb91a25e0535/pkg/controller/volume/persistentvolume/pv_controller.go#L478 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb4...
l0b0 2y ago
Some of this looks real bad though, like `if !found … else …`. Why the double negative, when you're going to have both branches anyway?
- theshrike79 2y ago
  You put the happy/hot path in the if case and the else is the abnormal/less usual path.
tim333 2y ago
There's quite an interesting discussion from Richard Hipp on converting sqlite code to aviation standards https://corecursive.com/066-sqlite-with-richard-hipp/#testing-and-aviation-standards https://corecursive.com/066-sqlite-with-richard-hipp/#testin... >DO-178B. It’s a quality standard for safety-critical aviation products... Your tests have to cause each branch operation in the resulting binary code to be taken and to fall through at least once... took a year of 60 hour weeks... It made a huge, huge difference. We just didn’t really have any bugs for the next eight or nine years.
wodenokoto 2y ago
> Every 'if' statement has a matching 'else' Is there an automated checker for this? I also have ad-box conventions for some code I write, but as long as there’s nothing but me between code and convention it’ll get broken right away.
exabrial 2y ago
People write code _not like this_ professionally?? This is 100% of our codebases. I think the only difference is we state that if feel the urge to write comment, write an [equivalent] log statement instead, so then we can use it in production for failure tracing.
jmartin2683 2y ago
It really feels like they’re just reaching for Rust and don’t know it yet
deterministic 2y ago
This is how important code should be written. No abstractions, with comments saying why things are implemented this way (instead of just saying what the code does). I wish more code would be written this way.
ThatUnknownDude 2y ago
I just realized I actually code somewhat like this

479 points by whalesalad 2y ago

// ================================================================== // PLEASE DO NOT ATTEMPT TO SIMPLIFY THIS CODE. // KEEP THE SPACE SHUTTLE FLYING. // ================================================================== // // This controller is intentionally written in a very verbose style. You will // notice: // // 1. Every 'if' statement has a matching 'else' (exception: simple error // checks for a client API call) // 2. Things that may seem obvious are commented explicitly // // We call this style 'space shuttle style'. Space shuttle style is meant to // ensure that every branch and condition is considered and accounted for - // the same way code is written at NASA for applications like the space // shuttle.

wouldbecouldbe 2y ago
hahaha well Kubernetes is the opposite of a special shuttle that keeps on flying. It crashes all the time, version updates etc. If you want stability go to apache or nginx.
- Sohcahtoa82 2y ago
  This comment is a complete non-sequitor. Kubernetes solves an entirely different problem from Apache and nginx.
  - deleted 2y ago
    [deleted]
  - wouldbecouldbe 2y ago
    yeah maybe it does, but when you have a hammer everything looks like a nail. Kubernetes is used all the time nowadays on simple projects just because it's the most intellectual pleasing solution. This is creating high maintainance costs by locking in companies with extremely complicated software where it's just not needed.
- andix 2y ago
  I've never seen Kubernetes crash. I don't have that much experience with operating k8s clusters, but on those I've seen it just kept on working.
  - whalesalad 2y ago
    Same. Pods will crash, run out of memory, fail to get scheduled, etc... but I have never seen kube itself crash.
    - q3k 2y ago
      I've been experimenting with k8s and/or running it on prod basically since the initial release, and I've so far only had one workload-affecting bug. The bug caused the scheduler to get stuck meaning pods assigned to nodes kept running, but no new pods would be scheduled. https://github.com/kubernetes/kubernetes/issues/124930 https://github.com/kubernetes/kubernetes/issues/124930 That's a pretty good track record, and indicates a level of fail-safe design (everything continued working, even though a critical components kept crashinglooping).
  - kesor 2y ago
    You obviously have not operated kubernetes long enough. With large enough scale you'll find the cluster controllers crashing for various reasons, getting out of sync with each other, etcd crashing or getting locked, and a whole bag of bugs.
- OJFord 2y ago
  If you can solve your problem with nginx then yes you should be using nginx not kubernetes...
- nimih 2y ago
  If you think nginx is the right tool for solving problems like container deployment, service discovery, cluster scaling, and secret management, then I suppose it's not surprising that you think Kubernetes "crashes all the time" and that a 1 year rolling support window for software releases is an insurmountable obstacle. Kubernetes has a lot of genuine issues and rough edges, but you're kind of showing your ass when you make comments like this.
  - wouldbecouldbe 2y ago
    There are just very few applications that actually need all of this. maybe 1 tot 0.1%, for instance vercel might need it. But 95-99% can just run on several "simple" servers & keep deployment times within minutes and no complicated stuff needed. Yet Kubernetes get's pushed all the time.
    - lyu07282 2y ago
      I don't know about that, I've found even simple applications need things like blue/green deployments and kubernetes makes that very easy and robust.
      - wouldbecouldbe 2y ago
        there is nothing easy or robust about kubernetes. Hence all the tooling around it, having things break down if you don't update. Dependencies not being compatible all the time. Server management should cost as little time, and be stress free. There are many ways to solve the up/down issue and depends on which language you are running.
        lyu07282 2y ago
        I agree if you are running kubernetes yourself you are absolutely right. I was thinking more about managed clusters in the cloud. Every provider offers managed kubernetes, then you aren't even vendor locked.
whalesalad 2y ago
I tried to link directly to these lines, but hn's url sanitizer stripped off the anchor tags.
arwhatever 2y ago
When I looked into Go I found it a bit surprising that someone had created a non-expression-based language as late as ~2009. I have not familiarized myself with the arguments against expression-based design but as a naive individual contributor/end-user-of-languages, expressions seem like one of the few software engineering decisions that doesn't actually "depend," but rather, designing languages around expressions seems to be unequivocally superior.
- TurningCanadian 2y ago
  instead of "expression" you meant "exception", right?
  - whalesalad 2y ago
    expression as-in s-expression (ex: lisp)
  - Jtsummers 2y ago
    In the statement/expression-oriented axis of languages, Go is a statement oriented language (like C, Pascal, Ada, lots of others). This is in contrast to expression oriented languages like the Lisp family, most, if not all, functional languages, Ruby, Smalltalk and some others. Expressions produce a value, statements do not. That's the key distinction. In C, if statements do not produce a value. In Lisp, if expressions do. This changes where the expression/statement is able to be used and, consequently, how you might construct programs.
    - runevault 2y ago
      A simple example for anyone who might not appreciate why this can be so nice. In languages where if is a statement (aka returns no value), you'd write code like int value; if(condition) { value = 5; } else { value = 10; } Instead of just int value = if(condition) {5} else {10} Some languages leave ifs as statements but add trinary as a way to get the same effect which is an acceptable workaround, but at least for me there are times I appreciate an if statement because it stands out more making it obvious what I'm doing.
supportengineer 2y ago
I went right into the code and looked for 'if' statements without 'else' statements. There are plenty. I don't see how you can have any exceptions to this rule if you are truly committed to capturing all branches.
- throwanem 2y ago
  If the 'if' condition matching always results in a thrown exception, a return, or likewise, then you don't really need an 'else' unless you're using a language which supports conditions and resumption (conformant Common Lisp implementations, and not really anything else I know of). The 'else', implicitly, is that the flow of control leaves the scope of the 'if' block at all. (I haven't read far enough into the code to know that this is what they're doing, but the head matter I did read suggests as much. It's a common enough pattern, especially around eg argument validation and other sanity checks a function might perform to ensure it can do meaningful work at all.) (I do wish HN supported an inline monospace markup, the <code> to a four-space indent's <pre>...)
  - iainmerrick 2y ago
    Swift's "guard" statement would be pretty handy here.
    - drewcoo 2y ago
      if (foo) { bail out } They're known as guard statements regardless of language. https://en.wikipedia.org/wiki/Guard_(computer_science) https://en.wikipedia.org/wiki/Guard_(computer_science) Swift has a guard keyword, but the construct feels a little awkward given most languages do the above. It makes me do a double take. guard !foo else { bail out }
      - iainmerrick 2y ago
        No, "guard" in Swift is special. Once you're in the "else" block, you MUST return or call a non-returning function (e.g. abort). It's specifically designed to prevent bugs where flow control accidentally resumes from an error handler. It's the same idea as the "every 'if' must have an 'else'" guideline in the code being discussed, except with "guard" the compiler will detect violations. It's a good thing.
deleted 2y ago
[deleted]

jerlam 2y ago

// KEEP THE SPACE SHUTTLE FLYING. I understand the intent, but it is a bit funny that the comment references a system that is no longer operational due to its poor safety record. In ten years or so, will people even remember the Space Shuttle in a good light?

whalesalad 2y ago
Well over 100 successful missions carrying a bunch of people and gear up into outer space and then bringing them back home. I hold it in a good light now, and will likely continue to feel that way. As far as human progress and net good, it was a success.
- joebob42 2y ago
  100 is frankly not that impressive for software. A 2-9 system will sometimes work 100x in a row, and a 3-9 system usually will.
  - rco8786 2y ago
    100 missions. Not 100 code executions.
    - joebob42 2y ago
      Sure, but I imagine at least some components only really execute a small number of times per flight, or possibly never in the case of certain error handling code. Stretching the metaphor more than is probably appropriate, I'd treat launching the shuttle and having it come back as a big integration test. A system that passes it's integration test 100 times isn't necessarily particularly impressive in terms of reliability. We run our integration test tens of times a day, and it fails once or twice a month. Our system is kinda flaky :(
      - rco8786 2y ago
        > A system that passes it's integration test 100 times isn't necessarily particularly impressive in terms of reliability. So extending your own metaphor and using 100 as the number of missions, the integration test failed 2% of the time.

vednig 2y ago

Main reason it is, as it is, and is usually not seen in any other languages is that go does not has operator functions for variable fallbacks and logical error handling. But overall this helps in improving performance of application(on a large scale).

boilerupnc 2y ago

Related article on Space Shuttle Software Quality [0] Excerpt: "But how much work the software does is not what makes it remarkable. What makes it remarkable is how well the software works. This software never crashes. It never needs to be re-booted. This software is bug-free. It is perfect, as perfect as human beings have achieved. Consider these stats : the last three versions of the program — each 420,000 lines long-had just one error each. The last 11 versions of this software had a total of 17 errors. Commercial programs of equivalent complexity would have 5,000 errors." [0] https://archive.is/HX7n4 https://archive.is/HX7n4

deleted 2y ago
[deleted]
emerongi 2y ago
5000 / 17 ≈ 295. Is it a fair assumption to make that a commercial program of equivalent complexity would take 295x fewer man-hours?
- fleischhauf 2y ago
  without any knowledge of the other processes involved in space shuttle software development, I highly doubt that the correlation is that easy
- burkaman 2y ago
  No, I don't think so, 295x is a crazy high factor. The article says 260 people are involved, and let's generously say it took 20 years to write the software (the first mission was 10 years after the program started and it was around for a total of 40 years). Dividing by 295 means a commercial team of the same size could have done it in less than a month. Or with a 10x smaller team, about 8 months. I don't think either of those are plausible.
  - deleted 2y ago
    [deleted]
  - swiftcoder 2y ago

philip1209 2y ago

Ah, 2k-line files makes me miss my days of coding in Go.

pizzafeelsright 2y ago
Join me. Still crushing it.
LAC-Tech 2y ago
2k lines is a whole widely used and feature filled open source library in many languages.
- stitched2gethr 2y ago
  Yes, but often density is at odds with readability.
  - LAC-Tech 2y ago
    As someone who has coded in languages of different density, I disagree. It's very easy to lose context and the big picture when you're scrolling around code written in a non dense language. (I'm talking the difference between say... Java and F#. I can't say much for the more extreme differences like COBOL and APL)
theshrike79 2y ago
It's a lot easier to glance through a single 2k line file than it would be to go through 200 separate 100 line files (+ extra overhead for imports in each file)

jsbg 2y ago

> // 1. Every 'if' statement has a matching 'else' (exception: simple error > // checks for a client API call) > // 2. Things that may seem obvious are commented explicitly Honest question: Why invent "safety" practices and ignore every documented software engineering best practice? 2,000 line long modules and 200-line methods with 3-4 if-levels are considered harmful. Comments that say what the code does instead of specifying why are similarly not useful and likely to go out of date with the actual code. Gratuitous use of `nil`. These are just surface-level observations without getting into coupling, SRP, etc.

anonymoushn 2y ago
Is there any evidence that these things are harmful or just vibes?
bayindirh 2y ago
Because sometimes, there's "No Other Way(TM)". Arbitrary line limits tend to unnecessary fragmentation. Add includes, licenses, glue code and comment; and you have an unapproachable spaghetti. Try to keep methods to 200 lines in high performance code, and see your performance crash and burn like Icarus' flight. When you read the comments in the code, you can see that they simplified the code to a single module, and embedded enormous amount of know-how to keep the code approachable and more importantly, sustainable. For someone who doesn't know the language or the logic in a piece of code, the set of comments which outline what the code does is very helpful. In six months, your code will be foreign to you, so it's useful for you, too. Comments are part of the code and the codebase. If you're not updating them as you update the code around them, you're introducing documentation bugs into your code. Just because the compiler doesn't act on them doesn't mean they are not functional parts of your code. In essence they're your knowledge, and lab notebook embedded in your code, and it's way more valuable in maintaining the code you wrote. They are more valuable than the code which is executed by the computer. Best practices are guidelines, not laws or strict rules. You apply them as they fit to your codebase. Do not obey them blindly and create problematic codebases. Sometimes you have to bend the rules and make your own, and it's totally acceptable when you know what you're doing.

ChrisArchitect 2y ago

Discussion from 2018: https://news.ycombinator.com/item?id=18772873 https://news.ycombinator.com/item?id=18772873

whalesalad 2y ago
wtf I submitted this in 2018?????
- pranshum 2y ago
  Our obsessions never change!
  - whalesalad 2y ago
    Nothing is quite as exciting as exploring the implementation of persistent volume claims
    - xarope 2y ago
      There's a Borges story lurking here...
- daedrdev 2y ago
  The joys of the human memory
- scj 2y ago
  That begs the question: What brought you to this file?
  - whalesalad 2y ago
    I saw it in the kube reddit, got a chuckle, and figured I would share here. Guess I got a chuckle 6 years ago too! Can't remember how I found it last time.
- deleted 2y ago
  [deleted]

EgoIsMyFriend 2y ago

Previous discussion https://news.ycombinator.com/item?id=18772873 https://news.ycombinator.com/item?id=18772873 (Dec 2018, 1552 upvote, 631 comments)

dangsux 2y ago
By the same user no less

renewiltord 2y ago

Why was Space Shuttle code so good and the engineering so bad? The thing was expensive and shit and had a 1.5% catastrophic failure rate for passenger transport. Soyuz was two orders of magnitude better. Russia/USSR have reputation for McGyvering things and US has reputation for gold-plating but US ship is killing people every 65 flights and Russian ship has over 1500 launches without death. Maybe engineers should learn from either Soviet/Russian engineers or from NASA software engineers. Both are making more reliable things.

teractiveodular 2y ago
Multiple books have been written on this very topic, but the TL;DR is that the problem was not the engineering, but the absurd, often mutually contradictory design decisions forced on it for political reasons. https://en.wikipedia.org/wiki/Space_Shuttle_design_process https://en.wikipedia.org/wiki/Space_Shuttle_design_process
- nvarsj 2y ago
  The irony is this kind of describes k8s as well.
  - Yasuraka 2y ago
    How so?
    - nvarsj 2y ago
      Hmm I guess my comment was kind of tongue in cheek. I think a lot of k8s was driven by political or pet-project reasons. E.g. using etcd.
ranger207 2y ago
https://waynehale.wordpress.com/2012/11/12/after-ten-years-a-few-words-from-admiral-gehman/ https://waynehale.wordpress.com/2012/11/12/after-ten-years-a... > I need to give you the issue from the NASA point of view so you can understand the pressures that they were under. In a developmental program, any developmental program, the program manager essentially has four areas to trade. The first one is money. Obviously, he can go get more money if he falls behind schedule. If he runs into technical difficulties or something goes wrong, he can go ask for more money. The second one is quantity. The third one is performance margin. If you are in trouble with your program, and it isn’t working, you shave the performance. You shave the safety margin. You shave the margins. The fourth one is time. If you are out of money, and you’re running into technical problems, or you need more time to solve a margin problem, you spread the program out, take more time. These are the four things that a program manager has. If you are a program manager for the shuttle, the option of quantity is eliminated. There are only four shuttles. You’re not going to buy any more. What you got is what you got. If money is being held constant, which it is—they’re on a fixed budget, and I’ll get into that later—then if you run into some kind of problem with your program, you can only trade time and margin. If somebody is making you stick to a rigid time schedule, then you’ve only got one thing left, and that’s margin. By margin, I mean either redundancy—making something 1.5 times stronger than it needs to be instead of 1.7 times stronger than it needs to be—or testing it twice instead of five times. That’s what I mean by margin. > It has always been amazing to me how many members of Congress, officials in the Department of Defense, and program managers in our services forget this little rubric. Any one of them will enforce for one reason or another rigid standard against one or two of those parameters. They’ll either give somebody a fixed budget, or they’ll give somebody a fixed time, and they forget that when they do that, it’s like pushing on a balloon. You push in one place, and it pushes out the other place, and it’s amazing how many smart people forget that.

neilv 2y ago

BTW, when linking to a file in GitHub like this, you can link to a range of lines of the file, by using the URL fragment identifier, like: #L60-L92 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb454ce69dee737e3eb91a25e0535/pkg/controller/volume/persistentvolume/pv_controller.go#L60-L92 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb4...

runlevel1 2y ago
You can click a line number to select the start of the range, then shift+click to select the end of the range to do this automatically.
ivanjermakov 2y ago
I think the whole file worth skimming through
whalesalad 2y ago
Try submitting a URL to HN that has an anchor tag and see what happens =)
corytheboyd 2y ago
I (well really, a coworker of mine) just today discovered a JetBrains action “Copy GitHub URL” [sic] that, if you have lines of code selected in the IDE, includes those lines in the copied URL fragment. So so so much better than my old workflow of stopping what I’m doing, going to the file in GitHub, and selecting the lines there to share links to bits of code.
- croemer 2y ago
  Same exists in VScode, probably need some GitHub extension for it.

kylehotchkiss 2y ago

Is this like a thing where people want to appear to be contributing to open source and remove all the 'unneeded' elses from a codebase?

dangsux 2y ago
[flagged]

quotemstr 2y ago

This sort of code strikes me as an ideal candidate for translation into a declarative, rule-based, table-driven system. Such a thing is more comprehensible and more verifiable than ad-hoc imperative if-clause-rich code. Messy code of this sort is usually a sign of a missing abstraction.

School-Cotton 2y ago
The Go ideology is basically to just write down all the code, in a more or less straightforward translation of what you would have written in C, and not to try to abstract anything.
- GuB-42 2y ago
  Which is an underrated ideology. I once had to go through some code that was objectively terrible. The guy who wrote was is a mechanical engineer, close to retirement at the time, and self-learned in programming. He had absolutely none of the background you can expect a professional programmer to have, and in particular abstraction seemed like a foreign concept to him. Have 50 buttons, each doing essentially the same thing, and you will see 50 copy-pasted blocks of code. Particularly ironic that he was using Java, a language known for its culture of abuse of design patterns and abstraction. But despite that huge mess that code was, it was surprisingly readable. You could look anywhere in the code and understand what it is doing. No calling though interfaces, when you see foo.bar(), you can just follow the symbol in your IDE and that's the instruction that will be run, and many times, there is not even a function, just code, thousands of lines of it, different cases are just dealt with ifs. Maybe it was the most pleasant "bad code" I had to work with. Code using the wrong or too much abstraction is much worse, because it is just as buggy and ugly, but you don't even know what to expect when all you see is an interface call where the actual code is in a completely different part of the software, and what connect the two is in yet another place. With more "factories", "managers", "dispatchers", etc... than actual logic.
  - globular-toast 2y ago
    If you saw the 50 copy-pasted blocks of code I'll bet you mentally abstracted it and assumed they were all doing the same thing. The problem comes when one of them isn't quite doing the same thing. That isn't possible with a real abstraction. Perhaps as a mechanical engineer the guy understood the more important things, though, like separation of concerns and a layered model, ie. architecture. Truly bad code mixes up all concerns into one ball of mud. Feel like forking a new process right from the GUI layer (the only layer) based on some business logic for that one button press? No problem! Being able to look at a single piece of code in isolation and understand what it's doing isn't the challenge. Anyone can do that for any code. The challenge is knowing how it runs in context of the larger program. What are the downstream implications of the way it's done? How many times is this run and why? Who is this code responsible to and why would it change (e.g. is it business logic, or just a UI thing)? This is the kind of thing an architecture gives you. You don't need to go all in with abstracting everything, but you do need some architecture.

copypasterepeat 2y ago

I've obviously only skimmed the code, but honestly it doesn't look that bad to me. Sure there are things I would do differently, but I've seen much, much worse. At least the code follows a single convention, and has the appearance that everything was thought through and that there is method behind the madness, as it were. I'd take this any day over the typical mishmash of styles, lazy coding, illogical code structure etc. that I've encountered so many times.

matthewmacleod 2y ago

Personally I love this level of verbosity in code. There are still way too many levels of nested control flow for my taste—I find that makes it exceptionally hard to retain context—but at least there are early returns.

theshrike79 2y ago
With a half decent editor you can fold them down so that just the comment on top of the if branch is visible

dpedu 2y ago

I wrote a toy kubernetes CSI driver recently and found it quite pleasing to do. Bare minimum means implementing just 3 grpc api calls - An informational one and publish/unpublish. Amazon's EFS or EBS CSI drivers are good examples because they're a pretty small codebase. I don't know exactly how this code interacts with the CSI driver itself, but it appears that it is the logic that ultimately results in the volume manipulation calls the controller makes against the CSI driver. It's nice that the complexity is all here, I was actually pretty surprised how simple the drivers themselves are.

wavemode 2y ago

Am I weird in feeling like the code in this file is really really... normal? Like, it's verbose in certain ways due to being written in Go, as well as due to not relying on any deep abstractions (and I don't mind this - abstractions are a double-edged sword), but in general, as code, it seems typical - and if the header text didn't exist I wouldn't think twice about the style it's written in. Maybe the disconnect here is that most of my experience is in enterprise software rather than systems software. Perhaps many of the comments in this file seem unnecessary to regular contributors within the k8s project? Whereas if I were writing this same code in an enterprise (and thus expect it to be read by people far in the future lacking context on all the technical details) I would have put -more- comments in this file, given the sheer complexity of all it's doing.

mumblemumble 2y ago
I wish code like this still felt normal to me, but over the past ~10 years it seems that many people have come to value brevity over explicitness. I strongly prefer the explicitness, at least for important code like this. More than once in my career I've encountered situations where I couldn't figure out if the current behavior of a piece of code was intentional or accidental because it involved logic that did things like consolidating different conditions and omitting comments explaining their business context and meaning. That's a somewhat dangerous practice IMO because it creates code that's resistant to change. Or at least resistant to being changed by anyone who isn't its author, though for most practical purposes that's a distinction without a difference. Unnecessarily creating Chesterton's fences is anti-maintainability.
- bhauer 2y ago
  Agreed. Explicitness and comments are very useful in understanding the intended functionality and logic, whether or not the code actually implements that intent correctly (an in providing that intent, they can help identify bugs earlier than they would be identified otherwise).
  - dskrvk 2y ago

BaculumMeumEst 2y ago

I do like the thought of firing Kubernetes into space.

sam_perez 2y ago

Beautiful. ---------------------------- This controller is intentionally written in a very verbose style. You will notice: 1. Every 'if' statement has a matching 'else' (exception: simple error checks for a client API call) 2. Things that may seem obvious are commented explicitly We call this style 'space shuttle style'. Space shuttle style is meant to ensure that every branch and condition is considered and accounted for - the same way code is written at NASA for applications like the space shuttle. ---------------------------- ^^^^ This bit reminds me of exhaustive checks in typescript code. I try to use them all the time. https://www.typescriptlang.org/docs/handbook/2/narrowing.html#exhaustiveness-checking https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

shepherdjerred 2y ago
You might like ts-pattern https://github.com/gvergnaud/ts-pattern https://github.com/gvergnaud/ts-pattern
bubblyworld 2y ago
Hah, years of writing ts for various contracts and I had no idea that never could be used like this. Thanks!
jeremyjacob 2y ago
The newer `satisfies never` is great for this purpose. It’s also convenient with if else chains if that’s more one’s style.

bsder 2y ago

When, oh, when, will we get a programming language that actually supports state machines properly ...

tashmahalic 2y ago
What does supporting state machines properly look like?
- bsder 2y ago
  Dunno. Maybe a table with functions and conditions? The point isn't the specific form that support would take. The point is that it has some form that implies the intent of the programmer. It has N states and they are enumerated. It has code that runs on transition and state. It has these variables which make up conditions. The state doesn't become a dead end and all states are reachable. etc. The point is to enable the compiler to know "Hey, this is a state machine. You can check <invariant> because it is a state machine.

zoogeny 2y ago

// CSINameTranslator can get the CSI Driver name based on the in-tree plugin name type CSINameTranslator interface { GetCSINameFromInTreeName(pluginName string) (string, error) } Do people actually find comments like the above useful?

deleted 2y ago
[deleted]
jakevoytko 2y ago
On a big open-source project like Kubernetes, they're probably happy with the tradeoff between "some exported names have inane and obvious comments" and "our linter requires open-source contributors to document their exported names in a consistent way."
sollniss 2y ago
With the official linter (golint), exported types must be commented, so sometimes you get Captain Obvious comments like this.
kmoser 2y ago
Yes, if it's your first day on the job and you don't know that CSI is a driver. Yes, if GetCSINameFromInTreeName ever gets renamed to something less obvious.
rk06 2y ago
yep, they are useful. I consider comments to live at "conceptual" level while code lives at "physical" level. With this way,when you are debugging, your mind can read code at conceptual level and easily disregard irrelevant blocks of code. without comments, i will need to mentally drop down at physical level and implement a compiler in my brain for the same results
akvadrako 2y ago
In this example it's faster for me to understand what the code does by reading the comment than to parse the code itself. If you're just scanning over a lot of code and looking for what you care about it could be helpful.

inamberclad 2y ago

If people want to look at some Real Deal Space Code, here's some parts I like: CoreFlight System's executive code: https://github.com/nasa/cFE/tree/main/modules/es/fsw/src https://github.com/nasa/cFE/tree/main/modules/es/fsw/src Any code from the core of RTEMS: https://gitlab.rtems.org/rtems/rtos/rtems/-/tree/main/cpukit/score?ref_type=heads https://gitlab.rtems.org/rtems/rtos/rtems/-/tree/main/cpukit... The code isn't special but it is neat and tidy, and very very clear. It says what it does, and it does something small and does it well.

CGamesPlay 2y ago

Is this code unit tested? If it's so critical that every branch has to be accounted for, I would assume that time would be well-spent combinatorically testing its inputs, right?

verandaguy 2y ago

Speaking specifically about cases where any not-completely-trivial `if` is matched with an explicit `else`: I wonder to what extent this code could be simplified if the authors of k8s had chosen to design around using structural pattern matching rather than `if`/`else` blocks? Lots of mainstream languages with support for structural pattern matching have compile-time tooling to check whether a match was exhaustive, which alone could serve as an idiomatic solution while increasing information density in the code.

deleted 2y ago

[deleted]

readthenotes1 2y ago

One part of what NASA did was have every line of code reviewed by many people, including someone whose sole job was to make sure the comments matched the code and vice versa. If you do not take such care, excessively for both comments are inevitably going to drift from the code and lead to incredible confusion.

geuis 2y ago

Joined a new company recently. They kept telling me their codebase was a mess. Yet I've been finding it to be remarkably refreshing. There are extensive comments everywhere. Lots of white space (remember to let your code breathe). Existing linting policies are extensive and thorough. The code is well structured and remarkably easy to follow through where that does what. I think it helps that it's a very small team with maybe 5-6 people max over time. It's such a pleasure to explore the project while learning the system and starting to fix some small long term issues. Really, really nice.

whalesalad 2y ago
This is a great sign because it shows humility of the engineering team.

wvenable 2y ago

I would consider this a code smell: if err != nil { return "N/A" } And I overall dislike the level of nesting in some of these functions but that might just be the nature of Go code.

nullc 2y ago

But do they have tests that achieve 100% condition/decision branch coverage? If they do, changes that disrupt handling some cases ought to be detected by the tests.

globular-toast 2y ago
I assume no and ultimately that is the point of code like this. As you say, if you can somehow correctly identify every possible branch in your tests then you could write the actual code any way you like. But then the tests would have to look like this, otherwise you'd have abstraction in your tests and you couldn't be sure if it covers every branch. Who tests the tests?
- nullc 2y ago
  You test the test by mutating the code. Once you have 100% condition/decision branch coverage you can automatically sweep the code with changes and the tests will fail. I have a little harness I use for this steps through each non-comment line of code changes signs, comparison directions, offsets values, swaps variables, adds negations, replaces computations with constants, etc. basically changes that are more or less guaranteed to compile. Then it runs the compiler to produce a stripped optimized output, if the compiler is successful, it checks that the resulting md5 is different from all the prior results, runs the tests. If the tests pass, it saves the passing code (which, to be clear is a meta-test failure), and then later I sweep through and either determine that it managed to produce functionally equivalent code or I improve the tests (and fix the resulting bugs they expose). The identical compiled binary test eliminates a lot of false positives. But that kind of approach doesn't work unless you get to ~100% condition/decision branch coverage since obviously any condition that isn't tested will be free to mutate. Hm. Maybe if I updated it I'll have some attempt at sampling LLM rewrites of functions. :P
  - globular-toast 2y ago
    Interesting approach. I hadn't considered actually implementing such "brute force" methods. I guess it's similar to fuzzing. I think the problem is you are then moving your "real" condition/decision branch documentation into your tests. The tests are then basically a guard rail for just in case someone modifies some abstract bit of code and it changes the behaviour of some otherwise opaque decision branch. The approach in OP seems to be to just move the "real" logic/documentation into the code itself and do away with the abstractions (and perhaps the tests too).

jmmv 2y ago

Nice. I wrote about a similar idea back in 2013 (https://jmmv.dev/2013/07/readability-explicitly-state.html https://jmmv.dev/2013/07/readability-explicitly-state.html) after I found that being explicit about all branches made my code easier to reason about and easier for reviewers to validate. Glad to find that this is “space shuttle style”! I’ve always disliked how the Go style insists on removing certain branches at the end of functions, for example.

raffraffraff 2y ago

It's an amazing code style for training LLMs.

edpichler 2y ago

When we have this on code is because we failed in something else.

serserser 2y ago

when i see too much verbosity i sometimes think someone is trying to hide something behind it...

dustedcodes 2y ago

I love this style of coding and it's something I've been doing increasingly more over the last few years. Code verbosity is so underrated in so many dev teams. I have never had a co-worker complain that my code was too well documented and too easy to understand, quite the contrary actually. When someone advocates for more concise code by saying that it's "easier and quicker to read" I always counter that if you use a lot of language feature magic to make code more concise then the mental work to read the code remains the same, the only difference is that you ask your co-workers to leave the editor and google lots of things in order to understand your brief code versus keeping them in their editor and just being able to read the simple verbose code without interruption in one place.

otikik 2y ago

This kind of thing should be enforced with linting rules and an automated action that rejects any pull request which violates the rules, not with a comment.

camgunz 2y ago

Can someone please get gofmt to wrap lines. This [0] is ridiculous. [0]: https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb454ce69dee737e3eb91a25e0535/pkg/controller/volume/persistentvolume/pv_controller.go#L478 https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb4...

l0b0 2y ago

Some of this looks real bad though, like `if !found … else …`. Why the double negative, when you're going to have both branches anyway?

theshrike79 2y ago
You put the happy/hot path in the if case and the else is the abnormal/less usual path.

tim333 2y ago

There's quite an interesting discussion from Richard Hipp on converting sqlite code to aviation standards https://corecursive.com/066-sqlite-with-richard-hipp/#testing-and-aviation-standards https://corecursive.com/066-sqlite-with-richard-hipp/#testin... >DO-178B. It’s a quality standard for safety-critical aviation products... Your tests have to cause each branch operation in the resulting binary code to be taken and to fall through at least once... took a year of 60 hour weeks... It made a huge, huge difference. We just didn’t really have any bugs for the next eight or nine years.

wodenokoto 2y ago

> Every 'if' statement has a matching 'else' Is there an automated checker for this? I also have ad-box conventions for some code I write, but as long as there’s nothing but me between code and convention it’ll get broken right away.

exabrial 2y ago

People write code _not like this_ professionally?? This is 100% of our codebases. I think the only difference is we state that if feel the urge to write comment, write an [equivalent] log statement instead, so then we can use it in production for failure tracing.

jmartin2683 2y ago

It really feels like they’re just reaching for Rust and don’t know it yet

deterministic 2y ago

This is how important code should be written. No abstractions, with comments saying why things are implemented this way (instead of just saying what the code does). I wish more code would be written this way.

ThatUnknownDude 2y ago

I just realized I actually code somewhat like this