lmi
[Top][All Lists]
Advanced

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

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


From: Vadim Zeitlin
Subject: Re: [lmi] [lmi-commits] master 2038991 15/33: Improve concinnity
Date: Thu, 13 May 2021 19:01:57 +0200

On Mon,  3 May 2021 08:15:53 -0400 (EDT) Greg Chicares 
<gchicares@sbcglobal.net> wrote:

GC> branch: master
GC> commit 20389914e2fa3ef518a75343c6c4d180f6ba8a41
GC> Author: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> Commit: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> 
GC>     Improve concinnity
GC> ---
GC>  ce_product_name.cpp          | 6 +++---
GC>  ce_skin_name.cpp             | 6 +++---
GC>  dbdict.cpp                   | 8 ++++----
GC>  rate_table_tool.cpp          | 6 +++---
GC>  wx_test_benchmark_census.cpp | 6 +++---
GC>  5 files changed, 16 insertions(+), 16 deletions(-)
GC> 
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, 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.

 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.

 Concinnity is important, but less important than clarity, IMO.
VZ

Attachment: pgpJlxwNMHJDo.pgp
Description: PGP signature


reply via email to

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