lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Greg Chicares
Subject: Re: [lmi] [PATCH] C++ m11n: range-based for loops
Date: Thu, 12 Jan 2017 10:24:22 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1

On 2017-01-11 22:39, Vadim Zeitlin wrote:
[...]
>               https://github.com/vadz/lmi/pull/52
[...]
>  Unfortunately, looking at it now, I realize that there are quite a lot of
> changes and it risks taking some time to review it, which I'm not sure you
> have.

I'll make the time. This changeset is vast, but fascinating.

Most of these changes are compelling improvements, e.g.:

-    typedef wxDataViewItemArray::const_iterator dvci;
-    for(dvci i = selection.begin(); i != selection.end(); ++i)
+    for(auto i: selection)
         {
-        erasures.push_back(list_model_->GetRow(*i));
+        erasures.push_back(list_model_->GetRow(i));
         }

That's much terser, and I always hated writing that '*i' asterisk.

Here's a similarly compelling change:

-    for
-        (std::vector<double>::const_iterator vi = v.begin()
-        ;vi != v.end()
-        ;++vi
-        )
+    for(double vi : v)

[although at this moment I tend to think 'for(auto const&' might
be better in both, as discussed in the "Request for review" thread]

Where clang-tidy proposes (e.g.)
-    typedef std::vector<Input>::const_iterator ici;
-    for(ici i = cell_parms().begin(); i != cell_parms().end(); ++i)
+    for(auto const& cell_parm: cell_parms())
I'll probably use a different name in the for-range-declaration,
because I think cell_parm and cell_parms are confusingly similar.

Lastly, on a comic note, at first I thought clang-tidy must have
gotten confused here:

-    typedef std::vector<std::string>::const_iterator aut0;
-    for(aut0 i = filenames.begin(); i != filenames.end(); ++i)
+    for(auto const& fn: filenames)

because no such name as 'aut0' could possibly exist in lmi...but it
must have been an abbreviation for "authenticity", and I guess in
2006 I wasn't thinking of 'auto' as a keyword (except as a nearly
forgotten artifact of K&R C).




reply via email to

[Prev in Thread] Current Thread [Next in Thread]