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

[Mouse]

>>> 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,
>> future-proofing.  If someone wants to change it in the future, it's
>> significantly easier to change one variable initialization than to
>> chase down everywhere that string is hardwired and check that each
>> one actually is referring to the same thing.
> While code flexibility is a good thing.  I have to wonder what would
> prompt the change of bc_program_stdin_name.

There may not be any.  I haven't dug enough to understand what the
variable actually means in context, which is one reason I didn't put it
any more strongly than "at least potentially reason to".

I certainly have written plenty of programs that read from stdin and
then, later, had occasion to change them to read from other files.  The
variable name looks as though it might (emphais: might) be a case of
something like that.  Based on what you've said, I consider that less
likely now.

> The only reason I could think of was internationalization, we already
> have a ton of English text in bc.c in the form of help text, keyword
> names, error messages, etc.

Which isn't necessarily a reason to add one more - though I suspect
<stdin> is less likely to be changed for i18n than more verbose things
like help text.

>> (It's easy to find all strings that say <stdin>. It's less easy to
>> verify that each of those is actually talking about the semantic
>> bc_program_stdin_name refers to.)
> The only time the string "<stdin>" appears in bc.c is the declaration
> for bc_program_stdin_name.

At the moment, sure.

Will that still be true in a month?  In a year?  In a decade?  I doubt
anyone has more than speculation on those questions.

>> If it were performance-critical enough that the cost of
>> dereferencing a variable were significant (and, for that small a
>> difference, if it were under my umbrella I'd want to see
>> measurements), then it might be worth doing.

> The compiler does liveness analysis and puts the variable in section
> .rodata, Although I can't confirm that since I haven't looked at the
> symbol table.

Then, I'm curious: what is your basis for saying it does so if you
haven't found it doing so?

> ""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;

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

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.

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.

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse at rodents-montreal.org
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B