Re: [lmi] [lmi-commits] master 0d3c75a 5/6: Improve readability

[Vadim Zeitlin]

On Tue, 28 Feb 2017 22:02:59 -0500 (EST) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit 0d3c75aaf160ee3a437685e5e02da255a6ac21e5
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Improve readability
GC> ---
GC>  input_sequence_entry.cpp | 12 ++++++------
GC>  1 file changed, 6 insertions(+), 6 deletions(-)
GC> 
GC> diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
GC> index 68d2a5a..f7ebc79 100644
GC> --- a/input_sequence_entry.cpp
GC> +++ b/input_sequence_entry.cpp
GC> @@ -492,6 +492,9 @@ std::string InputSequenceEditor::sequence_string()
GC>  
GC>          s.append(value_field(i).GetValue().c_str());
GC>  
GC> +        auto endpoint = duration_num_field(i).GetValue();
GC> +        std::string z = value_cast<std::string>(endpoint);
GC> +
GC>          switch(duration_mode_field(i).value())
GC>              {
GC>              case e_retirement:
GC> @@ -501,20 +504,17 @@ std::string InputSequenceEditor::sequence_string()
GC>                  break;
GC>              case e_attained_age:
GC>                  {
GC> -                s.append(" @");
GC> -                
s.append(value_cast<std::string>(duration_num_field(i).GetValue()));
GC> +                s.append(" @").append(z);
GC>                  }
GC>                  break;

 Wouldn't this code be even more readable if the names of "endpoint" and
"z" variables were exchanged? The current "endpoint" could well be made an
"unnamed" variable as it's only used once, and very close to its
definition, but using "z" for the string value doesn't seem very helpful to
me as it's used in several places below, relatively far from the place
where it is defined, and its meaning is not immediately clear there.

 I would also make both of these variables "const", but I'm not sure what
do you think of the "make all immutable variables const" rule that I
advocate.

VZ