Re: [lmi] [lmi-commits] master 2038991 15/33: Improve concinnity

[Greg Chicares]

On 5/13/21 5:01 PM, Vadim Zeitlin wrote:
> On Mon,  3 May 2021 08:15:53 -0400 (EDT) Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
[...]
> GC> commit 20389914e2fa3ef518a75343c6c4d180f6ba8a41
[...]
> GC>     Improve concinnity
[...]
> GC> diff --git a/ce_product_name.cpp b/ce_product_name.cpp
> GC> index 9e829be..2cea0da 100644
> GC> --- a/ce_product_name.cpp
> GC> +++ b/ce_product_name.cpp
> GC> @@ -42,13 +42,13 @@ std::vector<std::string> fetch_product_names()
> GC>  {
> GC>      fs::path path(global_settings::instance().data_directory());
> GC>      std::vector<std::string> names;
> GC> -    for(auto const& de : fs::directory_iterator(path))
> GC> +    for(auto const& i : fs::directory_iterator(path))
> 
>  I'd just like to note that I disagree with this. "i" is not an iterator
> here, unlike in a loop using begin/end,

If we could pick a good single-letter name other than 'i'
for all these not-an-iterators, I'd be happy to change
them all. I'd suggest 'q' because it's not often otherwise
used.

> although it's obtained from the
> class directory_iterator (IMO it would have been better to have a function
> with a more clear name returning this iterator, e.g. get_directory_entries
> or something like that). It is really a directory_entry and calling it "de"
> makes this much more clear.

It took me a while to guess that "directory entry" was meant.
If it were this:
  for(auto const& q : fs::directory_iterator(path))
then it would speak more clearly to me: 'q' would mean
  "the thing we're iterating with"
and it could be the same in all such loops. Not
  "the directory entry we're iterating with"
in one place, and
  "the string we're iterating with"
elsewhere: just "the thing", like the pronoun "it" (but 'it'
isn't a good name here because it suggests "iterator", and
this is a not-an-iterator).

I tend to dislike names with only two characters. They try to
say more than one character can convey, but an extra character
usually doesn't help much. A considerably longer name can
indicate its own meaning, though it can become balky if it's
used in many statements. A one-character name deliberately
has no meaning except the obvious one.

>  I know that we use "i" for not-iterators in the other places, but we also
> use other names in a couple of fields, and looking at the code involving
> both using "git grep 'for\(.* : '", I know which of them I find more clear.

If they can say what they mean, clearly and unambiguously,
in just a few characters or less, and their names truly
help make the code clearer, that's fine with me--e.g.:

- some short names are evocative and precise

  for(auto const& child : x.elements())

- some two-character combinations are so pregnant with meaning
that a search engine defines them in the first hit (or, at
least, the first hit that isn't a Korean rapper):

  input_sequence_entry.cpp:    for(auto const& rm : to_remove)

- this name's used at four widely-spaced points in a hundred-line
  loop, so it's worth welding two very short words together:

    for(auto const& run_basis : RunBases)

But consider these two examples from 'group_values.cpp':

    for(auto const& i : cells)
    for(auto const& ip : cells)

In the first one, 'i' (or, e.g., 'q' if you will) has
too few characters to mean anything, so it must mean
the only thing it can mean: "this thing". In the second,
what's 'ip'? Maybe it was originally "Input Parameters".
How can anyone guess? Why should anyone have to guess?
Search engines would say "Intellectual Property" or
"Internet Protocol". The one-character name is much
stronger.