Re: [lmi] [lmi-commits] master 6335e9a 1/4: Render comprehensible

[Greg Chicares]

On 2017-01-26 01:27, Vadim Zeitlin wrote:
> On Thu, 26 Jan 2017 01:17:21 +0000 (UTC) Greg Chicares <address@hidden> wrote:
> 
> GC> branch: master
> GC> commit 6335e9afadf5c5a4e6e9e98521a9653f2a9c67fe
> GC> Author: Gregory W. Chicares <address@hidden>
> GC> Commit: Gregory W. Chicares <address@hidden>
> GC> 
> GC>     Render comprehensible
> 
>  I'm not sure if you find C++11 vararg templates comprehensible...

Today I was just trying to bring this up to the era of TCP++L-1 or the
ARM: "We Built This City" or "Ice Ice Baby". This isn't development,
it's archaeology.

> ... but the functions check1() and check2() could clearly be combined into
> a single function using them, which would, IMHO, be nicer because these not
> completely trivial functions seem to be identical otherwise.
> 
>  I'm not sure if check0() could be _easily_ folded into the same vararg
> template however, at least with C++11 (with C++17 and its fold expressions
> it would be trivial).

Then I think this could wait for C++17. One checkN function for each
unique number of defaulted ctor args tested is a simple concept, and
replacing N functions with N-1 isn't as good as going all the way to
one. OTOH, perhaps check0() might just as well do what the others do,
and create an 's' which would probably have to equal an (empty) 'c';
then we could get to one. If you feel like doing that, please share
a patch.

As for me, I look at
  $grep 'TODO ??' input_seq* |wc -l
and see an opportunity to remove 26/422 = six percent of lmi's marked
defects. I couldn't modernize the "while" and "for" statements without
unravelling puzzles like this:

    // TODO ?? Not correct if last interval-endpoint specified is not the 
latest of all.
    if(intervals.size()) // TODO ?? And if not?
        {
        intervals.back().end_duration = last_possible_duration;
        intervals.back().end_mode     = e_maturity;
        }

but now I know the answers, and it'll never again be as easy to
clean this stuff up if I don't do it now.

Perhaps this class really wants delegating ctors: there's common
post-initialization in realize_vector(). Curiously, the snippet
above occurs on only one ctor; I know that now, because the
refactored code can rely on this as an invariant:
        intervals.back().end_duration = last_possible_duration;
but not this:
        intervals.back().end_duration = last_possible_duration;
And I've even discovered the reason for
    if(intervals.size()) // TODO ?? And if not?
simply by suppressing it and running the newly comprehensible unit
test (reason: " " is valid input).

Just please don't propose refactoring the parser, because it was
inspired by one of BS's older books, which was inspired by the
Dragon Book ("Don't It Make My Brown Eyes Blue"), and it's probably
clearer to leave it looking like the original. It would be nice,
though, somehow to sequester the variables that are shared by the
parser routines but not accessed by anything else.