[Toybox] [PATCH] bc.c: deobfuscation of bc_lib

[Rob Landley]

On 2/27/24 15:22, Mouse wrote:
>>>> And also, bc_program_stdin_name never changes from "<stdin>" so
>>>> there's no reason to make it a variable
>>> Well, I [...] would say that there is at least potentially reason to
>>> leave it as a variable even if it's never changed: namely,

Building domain expertise around bc is contingent on nontrivial users of bc
existing outside of the kernel I already have a patch for. And even if one exists:

  "Maybe it's something we can transplant." - Captain Terrell
  "You know what she'll say." - Pavel Checkov

>> ""Hiding numbers that are used just once into defines to put them out
>> of sight does not really help readability." - Pavel Machek"
> 
> Sort of.
> 
> I would say there is significant value in
> 
> 	#define MIN_INTEREST 10
> ...
> 	 if (nfound < MIN_INTEREST) return;
> 
> over
> 
> 	 if (nfound < 10) return;

Comments exist:

  if (nfound < 10) return; // min interest

> even if that is the only place MIN_INTEREST is used.  (At least,
> assuming the Eliza-effect meanings of "MIN_INTEREST" and "nfound" are
> actually appopriate to the problem domain.)

Which you don't know, because the macro name has to be small enough to be
usable, while a comment can be a whole line if that clarifies things.

Meanwhile, the value actually being compared is separated from the comparison,
and you have to switch over to look it up. (Maybe an IDE that's indexed the code
and shows you the definition when you hover or double click will help you, but
assistive devices aren't superior for not having the problem in the first place.)

> Of course, the difference is far less if the code is
> 
> 	 // 10 = min interesting count
> 	 if (nfound < 10) return;
> 
> but, even then, there is significantly more risk for the code and the
> comment to get out of sync than with the #define.

Oh no, the #define can be WALRUS_BLUBBER and still get used, telling you
nothing. The #define value can also make no sense whatsoever for the comparison
because we changed the units, but not get noticed because it's way the heck away
in another file and got missed in the conversion.

> There is also value in having the #define of MIN_INTEREST grouped
> together with a bunch of other relevant #defines instead of them being
> magic numbers scattered all over, even if each one is used only once
> and the uses have explanatory comments.

There is negative value in separating the data from the user.

"Where's the switch that controls the light next to my desk?"

"We put all the switches together in a common fusebox, it's down in the
basement. If you want to control any lights in the house, go down there and read
the labels, it's so much tidier that way."

Rob