Hacker News new | ask | show | jobs
by _wmd 4567 days ago
Encountered this topic recently in the office.. I'm from the side of the fence that doesn't overly care about code tidyness, so long as it performs its overall function. 10 years ago this kind of thing might have bothered me a lot more, but at some point crossed a threshold where I realized _all code generated to the present day_ is pretty ugly and long term unmaintainable (but that's a story for a rather large and rather boring essay).

The tl;dr is simply that if you obsess over minor details on this level, a lot of brainpower is wasted that could be used for bigger problems you should be much more worried about.

Playing devil's advocate, in this case the if() is obviously a guard for the subsequent switch. Moving just the constant '15' into a #define would make it read more like some magical sentinel value, unless you also #defined all the literal values used in the switch, at which point you've introduced a wholly bullshit layer of abstraction to what was otherwise incredibly concrete and explicit code.

Let's assume you have a great reason for doing that. OK. So what do you call these constants? Well, the code appears to be branching to special cases based on the width of some integer. So we instead have what, WIDTH_1_BIT, WIDTH_2_BITS, ..., WIDTH_15_BITS? Now we've pulled those constants out, you stare at the block of #defines, and think, damn, this is so ugly since most of the value space isn't fully defined! So some kindly maintenance programmer comes along and pads out the rest, producing a perfectly beautiful block of utter line noise.

That is arguably considerably less readable than what we started with

3 comments

I'm not sure how much this applies to open source. Closed source, quite possibly, since it probably won't survive long enough / need to change enough / need enough people to understand it.

Open source, meanwhile, requires being understandable to new people. It needs to explain itself, or it needs to be idiomatic (within its specialty). If neither, you're condemning people to rewrite it or waste time trying to figure out the original intent.

The second half of what I said explained how the code becomes less readable. If someone wants to hack on that code, they better understand the algorithm it implements, which means at a minimum they've probably absorbed the same 2000 page spec the original developers absorbed (and know exactly what 15 means). No amount of #define can fix that, although in the right circumstance some comments might help.
Your first paragraph sounds quite defeatist. In my experience, the way you deal with "minor details" is to write them down in a style guide. It'll take away a lot of the quibbling about small things. Some will find it limiting, others will find it liberating.

I've worked at several jobs that had decade old code in production. Some cared about the little things, some didn't. I know which code was the best to work with. In my experience, the broken windows theory is true when it comes to code. The little things matter in the long run.

A simple //comment would have sidestepped all of that and still clarified the number's purpose.
It's possible the original code had a //comment and it was stripped when they created the F/OSS repository.
This is a very insightful comment. There has also been some debate about the appropriateness of non english comments and comments that only made sense in context of internal code names for Cisco projects. The high level test we used was lets get the basic code out on github as early as possible.
It's all very well when it's a simple variable but quite often I have come up against variables that would take a whole paragraph of text to explain their purpose and don't have a concise or obvious name.
I find quite often a little hard to believe. Occasionally, maybe; and why not provide a paragraph of text? Sometimes it's the right thing to do.
Then write a paragraph of text. An hour writing today will save weeks of reverse engineering later.