Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Please do not attempt to simplify this code

github.com

Post reply on HN
Print view View original

311–320 of 327 posts

Re: Please do not attempt to simplify this code

#311

Post by mook »

irthomasthomas wrote: post #300
mook wrote: post #221

Earlier quoted context omitted.

Outdated comments are great, because it means you probably have a bug right there. If the comment didn't get updated, the code change probably didn't look at all the context and missed things. Pretty sure I'm guilty of that pretty often.

Looking at someone else's code, how would you know which was out of date, the code or the comment?

Does it matter? If the comment doesn't match the code, there's a bug (in the comment or the code). Either way you need to spend time to understand the context and figure out the correct thing, not trusting either.

Re: Please do not attempt to simplify this code

#312

Post by croshan »

omoikane wrote: post #149
wavemode wrote: post #98

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 her…

Most likely, this comment was added in response to a botched attempt to simplify code, to serve as a warning for future maintainers to think twice before making a similar attempt. The commit that added the warning was "Add note about space-shuttle code style"[1], and the one before that was "Revert controller/volume: simplify sync logic in syncUnboundClaim"[2] [1] https://github.com/kubernetes/kubernetes/commit/de4d1…

Nice find. I think you're right.

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.

Re: Please do not attempt to simplify this code

#313

Post by ElFitz »

spc476 wrote: post #223
ElFitz wrote: post #213

Earlier quoted context omitted.

That would be the purpose of formal proofs, wouldn’t it? Formal proofs may not be silver bullets, and we’re never safe from a faulty implementation of the proven algorithms, but this quanta article on a DARPA project showed impressive results [0]. There’s also AWS’ use of TLA+ [1]. [0]: https://www.quantamagazine.org/formal-verification-creates-h... [1]: https://news.ycombinator.com/item?id=22082869

"Beware of bugs in the above code; I have only proved it correct, not tried it." ---Donald Knuth

I will remember that one.

Re: Please do not attempt to simplify this code

#314

Post by adrianN »

odyssey7 wrote: post #309
adrianN wrote: post #248

Earlier quoted context omitted.

The system it runs on is part of the specification. A program is correct if fulfills all specified requirements. You're saying a car is defective because it breaks when you put sugar in the tank.

This is a no true Scotsman fallacy. Any time it runs incorrectly, it was invoked with the wrong operating conditions — which seem to be defined as any conditions that cause it to run incorrectly. 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…

You typically write down the specification before claiming that your code is done so you can’t use it to claim that any undesired behavior is not a bug. I naturally agree that in general tests are insufficient to show correctness because the state space is of impractical size, but, as I said above, for certain programs they totally can be.

Re: Please do not attempt to simplify this code

#315

Post by Shocka1 »

spacemadness wrote: post #132
mumblemumble wrote: post #115

Earlier quoted context omitted.

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…

Every time some code reviewer comes into my PR and says something along the lines of "you know you can just write it this way" where "this way" means obfuscating the code because "clever" and "shorter," I die a little on the inside. This is from experienced devs who should know better. At one point I wrote a comment write above a section I knew would be targeted by this kind of thinking explaining it must be written…

In one of my first positions out of undergrad we had a few devs on the team that got overly caught up in stuff like this. They were no doubt smart people, but their egos got in the way of things way too often. I'm not kidding - we'd get caught up on an if statement curly bracket for a ticket and there would be an argument for 30 minutes to an hour over whether the curly bracket should be there or not. These arguments would go into full blown dissertations, evolving into tangents of BSD coding style or Doom code. Keep in mind this was on a very well known automotive software platform with 20k bugs and counting open.

There is definitely a time and a place. Those were extremely frustrating times and a great learning experience for a young dev.

Re: Please do not attempt to simplify this code

#316

Post by jsbg »

high_na_euv wrote: post #257
jsbg wrote: post #16

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

> 200-line methods with 3-4 if-levels are considered harmful. Maybe if you are in love with software evangelists (bullshitters) like Uncle Bob

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

Re: Please do not attempt to simplify this code

#317

Post by joebob42 »

rco8786 wrote: post #263
joebob42 wrote: post #48

Earlier quoted context omitted.

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 reliabil…

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

A 2% failure rate isn't impressive, but I'm fine not crediting any of the shuttle issues to software. My only point is that 100 instance of use for purpose isn't enough, to my mind, to argue that a piece of software is exceptionally reliable.

Re: Please do not attempt to simplify this code

#318

Post by high_na_euv »

jsbg wrote: post #316
high_na_euv wrote: post #257

Earlier quoted context omitted.

> 200-line methods with 3-4 if-levels are considered harmful. Maybe if you are in love with software evangelists (bullshitters) like Uncle Bob

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

Doubtful 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?

Re: Please do not attempt to simplify this code

#319

Post by NiloCK »

jsbg wrote: post #81
cellularmitosis wrote: post #63

Earlier quoted context omitted.

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

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

John Ousterhout's Philosophy of Software Design is a good source describing the tradeoff analysis that suggests this approach.

https://milkov.tech/assets/psd.pdf

Post reply on HN