Re: [lmi] [PATCH] C++ m11n: range-based for loops

[Vadim Zeitlin]

On Thu, 19 Jan 2017 23:04:02 +0000 Greg Chicares <address@hidden> wrote:

GC> I'm done with
GC>   
https://github.com/vadz/lmi/pull/52/commits/d1fd3ae2e222c6d7a9edd1c9124ef2935e2918a5
GC> and have pushed the final changes.

 Thank you!

 I'm really happy with these changes, after working with C++11 in my other
projects it was really painful to force myself to write (or even read)
"for" loops using all these long iterator names in lmi code base.

 One small thing I'm still not completely satisfied about (I have a real
lack of talent for unconditional happiness, as you might have noticed) is
this idea that we shouldn't be using "for(auto& foo : foos)" because "foo"
and "foos" names are too close. IMHO this is the most natural way of naming
things (supposing that you use something a bit more descriptive instead of
a literal "foo") and the extra cognitive burden of having to remember that
"i" is actually a "foo" is much worse than any, non-existent IMO, as the
compiler will always catch it, potential for mixing up "foo" and "foos".

GC> Thanks for providing two separate patches. That was especially
GC> helpful for 'group_values.cpp': the first patch was easy, but the
GC> second was not. You may recall that you moved an int variable 'j'
GC> outside an old-style for statement. Careful study showed that the
GC> program is defective with that change; but more careful study
GC> revealed that it was identically defective without the change,
GC> and has been so since 20050416T0206Z.

 Sorry about this, I tried to preserve the existing code behaviour without
worrying too much about whether it was correct or not, maybe I should have
been more diligent.

 Anyhow, thanks again for applying these changes!
VZ