>>> 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 mo...@rodents-montreal.org / \ Email! 7D C8 61 52 5D E7 2D 39 4E F1 31 3E E8 B3 27 4B _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net