5 ms·
> // 1. Every 'if' statement has a matching 'else' (exception: simple error > // checks for a client API call) > // 2. Things that
by 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 agoIs there any evidence that these things are harmful or just vibes?
- bayindirh 2y agoBecause 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.
- johnnyanmac 2y ago> Try to keep methods to 200 lines in high performance code, and see your performance crash and burn like Icarus' flight. 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".
- bayindirh 2y ago> Are these loops in Kubernetes so hot that extra microseconds for some program stack manipulation will affect performance? 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 https://git.sr.ht/~bayindirh/nudge/tree/master/item/nudge.go
- ajuc 2y agoThere's nothing inherently wrong with a 200-line-long method. If the code inside is linear and keeps the same level of abstraction - it can be the best option. 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).
- deleted 2y ago[deleted]
- slaymaker1907 2y agoI tried writing in this "safe" way for quite a while, but I found the number of bugs I wrote was much higher and took way longer than just using railroad-style error handling via early returns. 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.
- mden 2y ago> Why invent "safety" practices and ignore every documented software engineering best practice? 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.
- ljm 2y agoThere is no single canonical suite of best practices. 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.
- johnnyanmac 2y agoIt's all opinions and "best practice" isn't some objective single rule to uphold. 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.
- kmoser 2y agoAssuming there is a compelling reason for a large file to begin with: with all due respect to VS Code and JetBrains, if the tools chokes because the file is too big, use a better tool. 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?
- jsbg 2y ago> There is no single canonical suite of best practices. 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.
- cellularmitosis 2y agoIf you think these things are considered harmful, I'd encourage you to read "John Carmack on Inlined Code" http://number-none.com/blow/john_carmack_on_inlined_code.html http://number-none.com/blow/john_carmack_on_inlined_code.htm... "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"
- jsbg 2y agoThanks. This is the first instance of a respected software engineer arguing in favor of this style that I have read (contrast with Dave Thomas, Kent Beck, Bob Martin, etc.)!
- NiloCK 2y agoJohn Ousterhout's Philosophy of Software Design is a good source describing the tradeoff analysis that suggests this approach. https://milkov.tech/assets/psd.pdf https://milkov.tech/assets/psd.pdf
- high_na_euv 2y ago> 200-line methods with 3-4 if-levels are considered harmful. Maybe if you are in love with software evangelists (bullshitters) like Uncle Bob
- jsbg 2y agoI would like to hear about what makes them bullshitters. I've had and seen really good results in terms of high productivity and low bug count on teams that followed the SOLID principles described in Robert Martin's Clean Architecture as well as Kent Beck's "make it work, make it right, make it fast." I've also universally observed the opposite results on teams that didn't.
- high_na_euv 2y agoDoubtful real world experience, Many stupid things came due to their work like "comments are bad" or ridiculous things like refactor of reasonably sized functions into very small functions - just a few LoC e.g 3. Ive seen Uncle Bobs refactor where he modifies thread safe code and introduces static properties to make code look elegant, but actually changes it behavior in multi thread environment, so basically didnt perform a refactor, but just introduced bugs But code looks better, so great thing to put into the book, right? >Kent Beck's "make it work, make it right, make it fast." How such a trivial thing can be even attributed to someone?
- gilbetron 2y agoSplitting a 200 line method into 20, 10-line methods rarely improves readability, it just tricks you into thinking those 200 lines are simpler than they actually are. 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.