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

161–170 of 327 posts

Re: Please do not attempt to simplify this code

#161

Post by marcus_holmes »

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…

I found myself thinking the same thing until I got to the hugely nested if statements. I would definitely have created some early-return branches to that thing.

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.

Re: Please do not attempt to simplify this code

#162

Post by marcus_holmes »

dskrvk wrote: post #137
bhauer wrote: post #119

Earlier quoted context omitted.

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

But comments go out of date, and the compiler doesn’t check them against the implementation. To document + enforce the intended functionality, use tests.

Comments (should) explain the "why" not the "what". The "why" doesn't go out of date, even if the "what" does.

Re: Please do not attempt to simplify this code

#164

Post by wavemode »

hyperpape wrote: post #160
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…

In particular, the every if has a matching else comment doesn't seem reliably true. Many of the unmatched ifs are just simple if (err != nil) { checks, but or other early returns, but outside of those, there do seem to be unmatched ifs. 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 c…

Yeah I should clarify that I was speaking personally about new code. I definitely encounter plenty of legacy code that is completely inscrutable.

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.

Re: Please do not attempt to simplify this code

#166

Post by jgwil2 »

dijksterhuis wrote: post #152
dskrvk wrote: post #137

Earlier quoted context omitted.

But comments go out of date, and the compiler doesn’t check them against the implementation. To document + enforce the intended functionality, use tests.

tests -> verify intended functionality implementation (the how is right). 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.

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

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.

Re: Please do not attempt to simplify this code

#167

Post by claytongulick »

mumblemumble wrote: post #115
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…

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…

> I strongly prefer the explicitness

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.

Re: Please do not attempt to simplify this code

#168

Post by inamberclad »

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

Any code from the core of RTEMS: 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.

Re: Please do not attempt to simplify this code

#169

Post by [deleted] »

mihaaly wrote: post #61
jerlam wrote: post #4

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

Since they used aluminium in the Space Shuttle would that also reflect poor safety record on using aluminium in mission critical situations?

[deleted]

Re: Please do not attempt to simplify this code

#170

Post by jakevoytko »

zoogeny wrote: post #163

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

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."
Post reply on HN