Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> The one thing that does bother me, however, is the name of the
> function, get_alpha_len(), which tells you (somewhat) literally what
> it does but doesn't convey to the reader its actual purpose (which is
> something we should strive for when naming functions and variables).
I actually think the helper function that is used as a building
block the "subcommand parser" uses should be named more directly
to represent what it does (i.e. look for a run of alphas) than
what it means (i.e. look for a run of letters allowed in a
subcommand name). IOW
char *end = skip_alphas(ptr);
if (*end == ':' && ptr != end) {
/*
* ptr..end could be a subcommand in
* "--fixup=<subcommand>:"; see if it is a known one
*/
*end = '\0';
if (!strcmp(ptr, "amend"))
... do the amend thing ...
else if (!strcmp(ptr, "reword"))
... do the reword thing ...
else
... we do not know such a subcommand yet ...
} else {
/* assume it is --fixup=<command> form */
...
}
conveys more information to readers than a variant where you replace
"skip_alphas" with "skip_subcommand_chars" without losing any
information.
Yes, in different contexts, where a helpers are designed to be used
by multiple callers that may not even be aware of each other, we do
encourage naming them after what they do _means_. But in this
codepath, I do not think it applies.
Thanks.
