undefined | Better HN

0 pointswilliamdclt0y ago0 comments

I was being humorous, but tbh it’s not so clear cut!

In 99% of cases, yes of course you’re right, factor this loop.

In this specific case? This is trivial code, that will likely _never_ change. If it does change, it’s extremely unlikely that the two loops would accidentally diverge (the dev would likely not miss one branch, tests would catch it, reviewers would catch it). So if you get any upside by keeping the two loops, it might be worth it.

Here you get 8 bytes back. I honestly can’t see how that would ever matter, but hey it’s _something_, and of course this is a very old program that was running on memory-constrained machines.

So it’s a trade-off of (minor) readability versus (minor) runtime optimisation. I think it’s the better choice (although it’s very minor).

Or maybe there’s a better reason they chose this pattern… can’t imagine the compiler would generate worse code, but maybe it did back in the days?

0 comments

layer80y ago

I agree that it’s borderline pedantic for this simple code, but I also find it an obvious code smell, contradicting the “as simple as it gets”.

If you consistently deduplicate code that is supposed to do the same and evolve the same, then any duplicated code sticks out as a statement of “this isn’t the same”, and in the present case it then makes you wonder what is supposed to be different about both cases. In other words, such code casts doubt on one’s own understanding, raising the question whether one might be overlooking an important conceptual reason for why the code is being kept duplicated. So in that sense I disagree that the duplicated version is more readable, because it immediately raises unanswered questions.

About possible performance reasons, those need an explanatory comment, exactly for the above reason. And also, if performance reasons warrant complicating the code, then it isn’t “as simple as it gets” any more. I was commenting because I disagreed with that latter characterization.

williamdcltOP0y ago

I agree it's a code _smell_. But a "smell" doesn't mean that something is necessarily wrong, just that there's a clue that it might be wrong.

> in that sense I disagree that the duplicated version is more readable

I didn't say it is, I agreed it's _less_ readable. I said it's trading off readability for 8 bytes of memory at runtime.

> If you consistently deduplicate code that is supposed to do the same and evolve the same, then [...]

I agree with all this. I'm not saying to consistently go for the deduplicated approach (I don't think anyone would say that), I'm saying it's a reasonable trade-off in this specific case (each branch is still trivial, and the code won't evolve much if at all).

> About possible performance reasons, those need an explanatory comment, exactly for the above reason.

Agreed.

> if performance reasons warrant complicating the code, then it isn’t “as simple as it gets” any more. I was commenting because I disagreed with that latter characterization.

Also agreed.

johnisgood0y ago

To some, the current way is more readable than yours, though. It is much more explicit, and that often is a good thing.

layer80y ago

I disagree that it’s more explicit. The case distinction and the loop and the puts are exactly the same in both code variants. You can replace the ternary operator by an if if that’s bothering you, that wasn’t the point of the change. The point is to first determine what should be output repeatedly, and then to output it, because the output logic is independent from what is being output (in particular, `yes` and `yes y` should be guaranteed have identical behavior). I don’t really see what’s non-explicit about that. Rather to the contrary, it makes it explicit that the output logic is intended to be independent from what is being output.

1 more reply

j / k navigate · click thread line to collapse

0 comments

layer80y ago

I agree that it’s borderline pedantic for this simple code, but I also find it an obvious code smell, contradicting the “as simple as it gets”.

williamdcltOP0y ago

I agree it's a code _smell_. But a "smell" doesn't mean that something is necessarily wrong, just that there's a clue that it might be wrong.

> in that sense I disagree that the duplicated version is more readable

I didn't say it is, I agreed it's _less_ readable. I said it's trading off readability for 8 bytes of memory at runtime.

> If you consistently deduplicate code that is supposed to do the same and evolve the same, then [...]

> About possible performance reasons, those need an explanatory comment, exactly for the above reason.

Agreed.

> if performance reasons warrant complicating the code, then it isn’t “as simple as it gets” any more. I was commenting because I disagreed with that latter characterization.

Also agreed.

johnisgood0y ago

To some, the current way is more readable than yours, though. It is much more explicit, and that often is a good thing.

layer80y ago

1 more reply

j / k navigate · click thread line to collapse