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.

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.

The "why" can go out of date, e.g. "do X before Y because [specific thing in Y] is dependent on [specific thing in X]". If you rewrite Y to no longer be dependent on X, the comment is now out of date.

The reality is that any comment can go out of date at any time if the code it describes changes enough. But that's really no excuse for failure to maintain comments. Sure, in reality code is messy and inconsistently written, not even counting comments. Comments are an essential parts of your codebase, and while they are used exclusively by humans, that doesn't mean they are any less worthy of being updated and cultivated.

Though, obviously, accidents happen, etc. But then that also happens with tests and everything else. I have definitely seen out-of-date tests in code bases, where the test is no longer relevant but still maintained.

Pretty sure I'm guilty of that pretty often.

comments -> why intended functionality was implemented that specific way (marketing wanted X because of Y, so we had to do it like Z with a bit of A).

> But comments go out of date

Just like updating the tests when code is changed, update the comment when the code is changed.

The same people who do the bare minimum for tests not to explode. But won’t add a new test case for the new branches they just introduced.

The same people who will mangle the code base introducing bizarre dependencies or copy paste the same piece of code rather than refactor.

People who fail to handle errors correctly. My favorite: by wrapping code in a if statement without an else. (else? Get a weird error without logs! Miles away from the call site!)

People who don’t validate inputs.

People who don’t give a monkey about adding context to errors making the thing impossible to debug in prod when they explode.

People who are too lazy or in incompetent to do their job properly and will always jump at the opportunity to save 5 minutes now but waste 5 hours of everybody else’s time later. Because of course these people can’t fix their own bugs!

And of course these are the people who make comments go out of date. I’ve seen them implement a thing literally the line below a TODO or FIXME comment and not delete the line.

Comments going out of date is a shit excuse for not writing comments as far as I’m concerned.

The fact that some people are incompetent should not drive engineering decisions. You should always assume a minimal level of competency.

I agree.

> Comments go out of date because of bad developers

I disagree.

Comments can also go out of date because

- developer is having a really shit time atm and their head is not in the game (bad looking after people management)

- developer is on a one day a week contract and doesn’t have the time in their contract to write big header comments explaining nuances (bad strategy)

- developer thought it looked obvious to them but it’s not obvious at review time (developer is being classic human)

- developer is getting pushed to deliver the thing now now now (bad workload management)

Most of those are the result of some decision made by someone who was not the developer (they’re all real cases). And they are the “non-code blockers” that good managers solve for us, so we can focus on getting good stuff done.

I’ve been where it seems like you are at. Blaming others for being bad didn’t help me. I had to lower my expectations of others, keeping my expectations for myself. Then get on about teaching willing people how they could be better. Might be making a few assumptions/projecting a bit there, but that’s my experience with “bad developers”.

Being any type of “leader” is lonely. Whether that’s an official role assigned to you or not. Or if it’s just a skill level thing. No one can quite match up to the expectations or understand what we see and why. But explaining it goes a long way with the right ones.

Well, yeah. But the point is that tests can be run in a pipeline that can fail if the tests fail. Comments going out of date has to get caught by a human, and humans make mistakes.

For example, "we have this conditional here because Business needs this requirement (...) satisfied for this customer"

Your comment can test the logic works correctly. But someone coming in, without the comment, will say "why are we doing this? Is this a bug or intentional? Is the test bugged, too?"

Now, they'll see it's intentional and understand what constraints the code was written under. Your test can't send a slack message to a business analyst and ask them if your understanding is correct. The original dev does that, and then leaves a comment explaining the "why".

While things like syntax are important, languages adding tooling like this (along with stuff like package managers) is so important to the continued evolution of the craft of software development.

Tests go out of date

Tests increase the maintainince burden

The compiler does not ensure code is tested

Tests get duplicated

Mēh! Tests matter, and testing is very important. Good judgment is required

Just like comments.

Writing code requires professional care at every step. The compiler helps of course see, but being professional is more than writing code that compiles

It involves documents too. And tests. Not too many (tests or documents) but not too few

Undocumented code is an enormous burden to maintain (I am eyebrows deep in such a project now). It is not enough to just write code and tests, documents including inline comments, are crucial

I never could understand that code. I know it wasn't brief, does that mean it was explicit?

I think the truth is brevity and explicitness are orthogonal. Let me ask you this: can code be both brief and explicit? What would that look like?

I like the tradeoff the Swiftlang project talks much about: brevity vs clarity (because explicit doesn’t necessarily mean clear, either, as your example shows). I think those are more orthogonal concerns, both important to think about, for different reasons that may often compete.

Otherwise, please optimize for writing stuff I can understand at 2AM when things have broken.

In your opinion, what problems might come from removing opinionated code reviews? Why do some reviewers gravitate toward “Here’s how I would have written it?”

But it could be that your team was just divided on approach and style.

They were struggling because they were trying to work out their differences through PR comments. That will be frustrating for everyone. Somebody went and got the PR working "the other way," and now the reviewer is trying to get the author to change the PR to "their way". If it goes on long enough, your devs will head for "the highway"...

If you just mandate "test pass = pr accepted," it will unblock your team short term, but in the long term, the large system will gravitate into many tiny, fiercely defended fiefs, each with different styles. Maintenance will be slow. Debugging will be complex. Wide-reaching refactors will be prone to blockage.

Fix the coding by having the team spend time "not coding." Establish a proposal and design process where an author needs to get team buy-in before they can implement. Establish an accepted standard on coding style. Do wide reviews on critical features and highlight issues where flow needs to go from one end of the system to the other and there are weird boundaries. Do the RCA (root cause analysis) and ask the "five whys". Look for patterns of issues and address them soon rather than pushing them to the back of the backlog. Prefer many small incremental changes over few large changes.

Each dev has their own way of writing things, their own little language. To them it is perfect. And it all works, passes tests. But, if you let them do this then your code becomes sloppy. Styles go in and out. Like reading a book where every other paragraph is written by a different person, and none of them talked to each other. Sometimes you get PascalCase, sometimes camel_case, sometimes snakeCase. Sometimes booleans are real bools, sometimes they're "Y" "N" (yes, real) or sometimes they're ints. Sometimes functions take in a lot of arguments, sometimes they take in a struct with options. Sometimes missing data is nullable, other times it's an empty string, othertimes its "N/A". And on and on.

You can enforce a lot of this through automatic linters, but not all. You require a set standard and the PR procedure can enforce the standard.

I have a rule for my teams: "Don't write clever code".

I try to constantly reinforce that we don't write code for ourselves, we write it for the next person.

We should be doing everything in our power to decrease their cognitive load.

I try to envision the person that comes after me (who may be me in months or years!) and imagine that they are having a Bad Day and they have to make changes to my code.

Good code is clear, and tells a story. A story that's easy to follow, and easy to drill into.

Not to knock elixir unfairly, but I think that's the basis of my mental block with that language. It seems to be designed from the ground up to violate that rule. Everything is elixir is clever. Very clever. Too clever for me.

Heck, it does't even need to be another person. Even me 10 months from now who may have forgotten some context around some code will appreciate boring code.

Even with ugly code, you should think about refactoring if this particular piece did run successfully for several years and there are no security related issues.

There are several languages the violate this principle. Brainfuck is probably one of the most prominent. It is of course not to be taken seriously. Overall alleged "elegance" of some code parts is rather annoying if you really need to understand and adapt it.

I have trouble with the equals sign being "pattern matching". There are other syntax things like that, where it seems like too much is being done in odd (to me) ways that are hard to grok.

I know a lot of people love it, and I really did try, but for whatever reason the syntax just doesn't work for me.

But Elixir is anything but implicit. If anything, people periodically raise a stink about wanting some magic in there, and we the wider community just reject them.

For example, I can't make heads or tails of rxjs, and honestly have zero motivation to do so.

Other people see it and find it intuitive.

Shrug.

FWIW, I don't think the code style in [2] is less simple (slightly more readable, to use `if (X) {} else {}` rather than `if (!X) {} else {}`, for example).

So to me, this reads as the author of [1] is just overcorrecting by adding process, when some test cases or code review would've been more helpful in preventing whatever incident [2] caused.

It does feel like the first "make it work" step (from "make it work, make it fast, make it pretty"), and then they just didn't do the "make it pretty" step. I have written code this "ugly" before, with this many comments, when working out a thorny state interaction. But I usually clean it up a bit before submitting for review. Maybe instead I should just put a huge "do not attempt to simplify this code" banner at the top hehe :)

But yes, it's not that bad, for sure.

That said, my experience in enterprise software isn't that extra comments are necessarily present (there was a plague of "// end if" comments in the codebase, but actual descriptive comments were rare).

But, I can say in 2024 at least, that most teams I've been on value explaining complex logic (especially logic that can break in subtle ways if not properly maintained) with comments, in new code we write.

My own code doesn’t look like that at all. I have long descriptive variables, what I think are easy to read and follow functions, etc. Sometimes I think if I want to be “good” I need my stuff to look like what I find, but at the end of the day, I want it to be easy for me (and hopefully others) to maintain. If no one else can read my code, I don’t view it as job security, I view it has handcuffs.

I know this is go code, but there's multiple places that go five branches deep, at least.

It works but it's a style that takes a lot of effort to grow what's happening IMHO.

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

What exactly do they mean by this? If each of the 3 versions had exactly one bug, isn't this just a weird way of saying the first 2 fixes either didn't work or introduced a new bug?

The SRR (software readiness review) process happened after development but prior to certification for launch. Most of the bugs were found here and were found to have existed in the code since the beginning of the program.

These were overwhelmingly low severity discrepancy reports.

If I recall correctly, there was a time when they were finding lots of bugs through SRR, so the main development team started their own "continuous review" designed to catch bugs before going to SRR. This made the SRR people angry because they were finding fewer bugs and felt the development team was focusing on competition over bug numbers rather than the code itself.

This reminds me of the Quality culture, at my last job, which was a famous Japanese optical corporation.

It was deliberately set up, so there was an adversarial relationship between QA, and Development, with QA holding the aces.

As a Development manager, it used to drive me nuts (Ed. Well, it wasn’t much of a “drive.” More like a short putt). It did result in very high-Quality software, but at the cost of agility.

It reflected their hardware development methodology, which regularly resulted in stunningly high-Quality kit, but I think caused a lot of issues with the usability and effectiveness of their software.

I rather it be built right, than quickly. I wish we held quality to higher standards in the software industry.

The claim that a codebase of 420k lines contains "only one error" is of course absurd, and the members of this forum would laugh anyone out of the room who made such a claim about any other project, pointing out how they cannot possibly know, actual logical contradictions in the claims as described by GP, or just plain ridiculing it without further elaboration.

But since the code in question cannot meaningfully be tested by the public, and people have been indoctrinated to believe the myth that aerospace engineers are an entirely different species that doesn't make mistakes in the sense that the rest of the workforce does, the hubris is accepted until disproven, which it probably won't be for various practical reasons.

Nevermind that the Space Shuttle was a death trap that killed two crews, and that the associated investigations (especially for Challenger) revealed numerous serious issues in quality and safety management. People will continue to just nod their head when they hear nonsense like this, because that's what they have seen others do, and so Schrödinger's Hubris can live on.

I doubt any of the people who actually write the code would stand by when that claim was made and not clarify it to 'we only found one bug'.

Many (most?) non-trivial bugs are actually flaws in the specification, misunderstandings about what exactly you wanted and what real-world consequences arise from what you specified.

Interesting to think about a formal code verification system that maintained a connection between all four prime artifacts: natural language description of problem as understood & intended solution, vs. code and the properties actually verified.

And then someone tries it with a ulimit of 16kB.

Does it run?

Do you _know_?

Do you even know what is correct behavior in this situation?

Sugar in the tank is an agreeable example because of how obvious it is, but what about something more subtle? An odd condition that leads to the wrong resonant frequency. An unknown software bug that makes the brakes lock up on Tuesdays at 12:00 AM on one specific backroad in a remote part of Virginia. The combinatorial possibilities of operating conditions are too numerous to exhaustively test.

I guess you could say that every function has every quality that it happens to have, so that functions need only exist in order to be “correct.”

Of course now the greatness of the feat depends on how much testing there was between versions, but given that it was the shuttle there was probably a lot.

Fixing a bug in 10,000 washing machine control boards is very expensive when it entails sending a technician to every house to replace the circuit board.

We didn't apply anywhere near that kind of quality control to smartphones or VR headsets. Once users are trained to install OTA updates to fix issues, most of the impetus for extreme quality control outside of the bootloader->OTA update path is gone

They have 3 relatively modern CPUs setup to run the same code and error check each other, such that if one has an issue, there's still redundancy when it's rebooting.

The software controlling critical systems is probably closer to NASA-esque practices.

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.

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?

the safety problems with the shuttle were, broadly speaking, hardware problems and not software problems.

from "Appendix F - Personal Observations on Reliability of Shuttle" [0], which was Richard Feynman's appendix to the report on the 1986 Challenger disaster:

> To summarize then, the computer software checking system and attitude is of the highest quality. There appears to be no process of gradually fooling oneself while degrading standards so characteristic of the Solid Rocket Booster or Space Shuttle Main Engine safety systems.

he specifically highlighted the quality of the avionics software as an example of how engineering on a project like the Shuttle could be done well, and wasn't doomed to be low-quality and unsafe simply by virtue of being a large complicated government project.

0: https://www.nasa.gov/history/rogersrep/v2appf.htm

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.

That’s not a great record. Sure it’s a complex field, and it’s not as dangerous as say being US President, but a failure rate of >1% is not something to write home about.

Not saying the Shuttle's success rate was awesome; I'm glad we demand more nowadays. But it still represented a pretty decent crew safety improvement for the USA's human spaceflight program.

We run our integration test tens of times a day, and it fails once or twice a month. Our system is kinda flaky :(

So extending your own metaphor and using 100 as the number of missions, the integration test failed 2% of the time.

Even though more astronauts died, because of the two shuttle accidents, than any other NASA disaster, the safety record was absolutely amazing, when we consider everything that was going on.

The code seems damn good code.

Are people aware of how old the technology is that's currently putting objects and people into space? No, the space shuttle was not obsolete. It was expensive... very expensive. To this day, we still don't have a replacement for it's capabilities though.

That's "the operation was successful but the patient died" logic. Killing over 1% of your riders is not a good safety record! No ifs, no buts.

However, the Space Shuttle had a much larger crew capacity than most missions probably needed (up to eight astronauts compared to Apollo or Soyuz's three), especially considering that the majority of Soviet/Roscosmos, ESA and CNSA missions were autonomous and completely unmanned - no crew to endanger!

Perhaps that makes the metaphor even better for Kubernetes: an highly engineered, capable and multi-purpose system requiring considerable attention, and probably used a little more than it should be.

We rarely flew the maximum number of passengers. On non-EVA missions we typically only sent up 5 astronauts. For EVA missions we usually sent up 7 with the two extra crew typically being dedicated to the EVA.

EVAs are a real chore. The shuttle is at 14.7 psi with regular atmosphere, but the EVA suits are 4 psi with pure oxygen atmosphere, so you have to spend a lot of time pre-breathing just to put on the suit. It also drains the hell out of you because it's microgravity, not zero gravity, and moving around and positioning in the suit using just your hands all day wears you out fast.

The extra capacity was also useful for bringing other nations personnel onto a mission with us. They didn't strictly have a large mission role, but it is good diplomacy, and helps other nations build up their own space program. Plus.. a few times.. we sent up 6 but brought back 7, which is a nice feature.

Anyways, when not sending up extra crew, we used the additional space for equipment and large experiment packages, some of them as large as a single person.

> and probably used a little more than it should be.

NASA did a great job of making space flight look normal and routine. Which is a bummer, because if you dig into any of their voluminous documentation on the shuttle program as a whole, or into individual missions, they are all anything but.

Space is just an absolutely insane environment to work in, let alone to discover and study from, and the shuttle was an excellent platform for that work. Somewhere along the way space commercialization became about building stations and just trucking people back and forth to them. And for that inglorious mission the shuttle is definitely not appropriate.

Anyways.. one of my favorite things about the orbiter.. the front thermal windows need to be tinted; obviously, but what I found out recently is they used the same green dye they use in US banknotes to do that.

Space is *insane*. I spent some time with satalite engineers. Space is far beyond hostile.

I loved 'For all mankind." Even with it's problems.

Per passenger mile traveled, the most common measure, it's one of the safest vehicles ever created and flown.

> will people even remember the Space Shuttle in a good light?

This is an honest question, since my childhood was squarely in the 1980s, but how can you possibly not? Are you so young that your only perspective of this program and all it's missions and accomplishments are purely in retrospect and colored by the zeitgeist of our current civilian space contractors?

Airliners are running 0.01 deaths per billion miles. Driving is 15 per billion miles. So it was worse than driving. Airliners beat it by 3 orders of magnitude. Which isn't surprising when US airliners travel distance of Space Shuttle in less than month.

So it's 5.1. Three times safer than driving. You might apocryphally conclude they were at greater risk taking the astro van to the pad.

Space flights were just side project...

Despite the safety record you mention, it was the best approximation of a spaceship that we, as a human species, ever created.

A spaceship program is something you either fully commit to by spending billions on it, or you abandon it.

Nevertheless, there has been nothing that has come close to it since.

>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.

----------------------------

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.htm...

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.

> // 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.

"The flight control code for the Armadillo rockets is only a few thousand lines of code, so I took the main tic function and started inlining all the subroutines. While I can't say that I found a hidden bug that could have caused a crash (literally...), I did find several variables that were set multiple times, a couple control flow things that looked a bit dodgy, and the final code got smaller and cleaner."

If Carmack finds value in the approach, perhaps we shouldn't dismiss it out of hand.

Also worth noting his follow-up comment:

"In the years since I wrote this, I have gotten much more bullish about pure functional programming, even in C/C++ where reasonable... When it gets to be too much to take, figure out how to factor blocks out into pure functions"

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.

Are these loops in Kubernetes so hot that extra microseconds for some program stack manipulation will affect performance? I never took Kubernetes as a hyper-real time application.

>Do not obey them blindly and create problematic code bases.

I don't know the code so won't question it specifically, but wouldn't this also apply to "space shuttle programming"? I feel Space shuttle programming's job in many ways is in fact to try and remove ambiguity from code. But not by explaining the language, but the variables and their units. I sure wouldn't mind spamming "units in cm" everywhere or explaining every branch logic if it's mission critical. Not so much this inconsistent doxygen/javadoc style documentation on every variable/class. If you're going to go full entrprise programming, commit to it.

Above everything else, the big thing going through my mind reading these are "a proper linter configuraion would have really helped enforce these rules".

Actually, looking at the code itself, pv_controller doesn't look overly hot, but extremely high value. In this case the long methods are intended to keep the logic confined, so one can read end to end and understand what is going on.

The code even doesn't use automatic type inference in Go (the := syntax), in some cases to make code more readable.

From what I understand, this code needs to be "kernel level robust", so they kept the overly verbose formatting and collected all three files to a single, overly verbose file.

I don't think this is a bad thing. This is an important piece of a scale-out system which needs to work without fault (debate of this is another comment's subject), and more importantly it's developed by a horde of people. So this style makes sense to put every person touching the code on the same page quick.

> I feel Space shuttle programming's job in many ways is in fact to try and remove ambiguity from code. But not by explaining the language, but the variables and their units. I sure wouldn't mind spamming "units in cm" everywhere or explaining every branch logic if it's mission critical.

A code comment needs to explain both the logic, and how the programming language implement this logic the best way possible. In some cases, an optimized statement doesn't look like what it's doing in the comment above it (e.g. the infamous WTF? comment from id Games which does fast_sqrt with a magic number). In these cases I open a "Magic Alert" comment block to explain what I'm doing and how it translates to the code.

This becomes more evident in hardware programming and while working around quirks of the hardware you interface with ("why this weird wait?", or "why are you pushing these bytes which has no meaning?"), but it also happens with scientific software which you do some calculation which looks like something else (e.g.: Numerical integration, esp. in near-singular cases).

> Not so much this inconsistent doxygen/javadoc style documentation on every variable/class. If you're going to go full entrprise programming, commit to it.

This is not inconsistent. It's just stream-of-consciousness commenting. If you read the code from top to bottom, you can say that "aha, they thought this first, then remembered that they have to check this too, etc." which is also I do on my codebases [0]. Plus inline comments are shown as help blobs by gopls, so it's a win-win.

I personally prefer to do "full on compileable documentation" on bigger codebases because the entry point is not so visible in these.

> "a proper linter configuraion would have really helped enforce these rules".

gopls and gofmt do great job of formatting the codebase and enforcing good practices, but they don't touch documentation unfortunately.

[0]: https://git.sr.ht/~bayindirh/nudge/tree/master/item/nudge.go

The problem with having an explicit else for every if block is that the complexity of trying to remember the current context just explodes. I think a reasonable reframe of this rule would be "Every if-conditional block either returns early or it has a matching else block". The pattern of "if (cond) { do special handling }" is definitely way more dangerous than early return and makes it much harder to reason about.

There is also nothing harmful or unharmful about the length of a function or the lines of code in a file. Different languages have their opinions on how you should organise your code but none of them can claim to be ‘best practice’.

Go as a language doesn’t favour code split across many small files.

But generally, best practices are "best" for a reason, some emperical. The machine usually won't care but the humans do. e.g. VS or Jetbrains will simply reject autocompletion if you make a file too big, and if you override the configuration it will slow down your entire IDE. So there is a "hard" soft-limit on how many lines you put in a file.

Same with Line width. Sure, word wrap exists but you do sacrifice ease and speed of readability if you have overly long stretches of code on one line, adding a 2nd dimension to scroll.

As for long lines, there is sometimes value in consistently formatting things, even if it makes it somewhat harder to read because the lines run long. For example, it can make similar things all appear in the same column, so it's easy to visually scan down the column to see if something is amiss.

In any case, since soft wrapping has been available for ages, why do you feel the need to reformat the code at all in order to see long lines?

There kind of is, though. Most software engineering books argue for the same things, from the Mythical Man Month to Clean Architecture.

> Different languages have their opinions on how you should organise your code

In general best practices are discussed in a language-agnostic manner.

Take a look at e.g goto

Using goto in C is normal thing

Using goto in C# web dev will get you a weird look during review

Using goto in C# stdlib dev is viable

So as you see good practices arent just tech dependent, but also product (and many more) dependent

Furthermore, how to split 200 lines into methods is context dependent. Looking through the lens of memory, optimality, simplicity, different flows of concern, and you'll want to split those 200 lines up differently.

The problem space is complex, hiding that fact doesn't get rid of that fact.

That seems unnecessarily brutal (and untrue).

> 2,000 line long modules and 200-line methods with 3-4 if-levels are considered harmful

Sometimes, not always. Limiting file size arbitrarily is not "best practice". There are times where keeping the context in one place lowers the cognitive complexity in understanding the logic. If these functions are logically tightly related splitting them out into multiple files will likely make things worse. 2000 lines (a lot of white space and comments) isn't crazy at all for a complicated piece of business logic.

> 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.

I don't think this is a clear cut best practice either. A comment that explains that you set var a to parameter b is useless, but it can have utility if the "what" adds more context, which seems to be the case in this file from skimming it. There's code and there's business logic and comments can act as translation between the two without necessarily being the why.

> Gratuitous use of `nil`

Welcome to golang. `nil` for error values is standard.

The alternative (let's say 40 5-line-long methods) can be worse (because you have to jump from place to place to understand everything, and you can mess up the order in which they should be called - there's 40! permutations to choose from).

    // CSINameTranslator can get the CSI Driver name based on the in-tree plugin name
    type CSINameTranslator interface {
        GetCSINameFromInTreeName(pluginName string) (string, error)
    }

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

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.

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.

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.

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)

    if err != nil {
      return "N/A"
    }

Messy code of this sort is usually a sign of a missing abstraction.

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.

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.

If you do not take such care, excessively for both comments are inevitably going to drift from the code and lead to incredible confusion.

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 because it is a state machine.

If they do, changes that disrupt handling some cases ought to be detected by the tests.

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

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).

        // ==================================================================
        // 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.

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.

Now every time I use typescript or go I have trouble trying to express what I want to say because `when` and similar expressions are just such a convenient way to think about a problem.

In go that means I usually end up extracting that code into a separate function with a single large `switch` statement with every case containing a `return` statement.

In most modern languages something like this could be implemented using composition with interfaces or traits. Especially in Rust it’s possible to write very robust code that has identical performance to the if-else spaghetti, but is proven correct by the compiler.

I’m on mobile, so it’s hard to read through the code, but I noticed one section that tries to find an existing volume to use, and if it can’t, then it will provision one instead.

This could be three classes that implement the same interface:

    class ExistingVolume : IVolumeAllocator
    class CreateVolume : IVolumeAllocator
    class SeqVolumeAllocator : IVolumeAllocator

Far more flexible and robust than carefully commented if-else statements!

Similarly, there's a number of "feature gate" if-else statements adding to the complexity. Let's say the CreateVolume class has two variants, the original 'v1' and an experimental 'v2' version. Then you could construct a SeqVolumeAllocator thus:

    allocator = new SeqVolumeAllocator( 
        featureFlag  ? new CreateVolumeV2() : new CreateVolumeV1(),
        new ExistingVolume() );

See the legendary Andrei Alexandrescu demonstrating about a similar design in his CppCon talk “std::allocator is to allocation what std::vector is to vexation”: https://youtu.be/LIb3L4vKZ7U

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.

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.

  (let [thing (cond
                pred-1 form-1
                ...
                pred-n form-n)]
    (do something with thing))

Trinary is expressly

let x = condition ? truevalue : falsevalue

You can do shenanigans by having something along the lines of

let x = condition1 ? truevalue1 : (condition2 ? truevalue2 : falsevalue)

More importantly, pattern matching is not necessary here.

  int value1 = 0;
  int value2 = 0;
  if (condition) {
    value1 = 8;
    value2 = 16;
  } else {
    value1 = 128;
    value2 = 256;
  }

  int value1 = if (condition) { 8 } else { 128 };
  int value2 = if (condition) { 16 } else { 256 };

let (value1, value2) = if (condition) { (8, 16) } else { (16, 256) }

Or else you’d just use some other sort of compound value like a struct or something. Tuple is just convenient for doing it on the fly.

Plus side I dabble in f# which is so much more expressive.

So like I believe you can do this in Rust (haven't written it in a while, I know it has destructuring of tuples)

let (a, b) = if (condition) { (1, "hello") } else { (3,"goodbye") }

My poor attempt at a definition, covers it in practice in languages I'm familiar, but not in theory, I assume: a language where switch statements return a value

    boo := bar
    if foo {
        boo = baz
    }

(I’m not exaggerating or being flippant: Google programmers being too stupid to understand modern programming languages has literally been cited as one of the main design goals of Go).

Anyway, complicated code should be avoided whenever possible, true, but banning the ternary operator (and similar constructs like match/switch statements as expressions) does nothing to make code simpler. It just forces you to transform

    let x = (some complicated nested expression);

    var x;
    // (some complicated nested tree of statements where `x` is set conditionally in several different places)

The `if` statement is just less ergonomic than the ternary operator, because statements don't compose as well as expressions do. A language which has a lazy ternary operator, and which lets you use an expression of type `unit` as a statement, does not require an `if` statement at all, because `if a then (b : unit) else (c : unit)` is identically `a ? b : c`. The converse is not true: you can't use `if` statements to mimic the ternary operator without explicitly setting up some state to mutate.

Check out the ‘case’ statement in elixir for an example.

In languages that support it, it usually becomes an incredibly commonly used expression because it’s just so applicable and practical.

(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 to a four-space indent's

...)

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.

A resumable exception initiated in the "then" part of an if will not go to "else" when control resumes; it will go to the next statement after the if.

One typically available restart is to ignore the condition and resume execution immediately after, as you describe. Another is to re-evaluate the form in which the condition was signaled. In that case, the conditional may well be itself re-evaluated with a different result, executing a different branch.

"Condition" is just a silly name for "exception". It does not mean "restartable exception". It's a terminology that Common Lisp copied from PL/I. At the time Common Lisp was being standardized, it was not a common programming language feature, so the naming didn't matter. In the decades since, the world went to "exception".

The word "condition" already has a clear meaning in computing, referring to a logical state ("condition control register", "conditional branch", ...). The "condition variable" synchronization primitive (which has no condition-like state!) is bad enough; we don't need to heap more meanings on those words.

Note that processor instruction sets have exceptions, precisely restartable, down to the instruction, without requiring a cooperating restart point. E.g. any code can hit a page fault: the exception handling will fix it up, making a page present at the faulting address, and then restart the instruction that faulted.

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

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).

There are many ways to solve the up/down issue and depends on which language you are running.