Re: [lmi] Delegating ctor uncalled for?

[Greg Chicares]

On 2017-02-04 02:16, Vadim Zeitlin wrote:
> On Sat, 4 Feb 2017 02:01:37 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2017-01-26 02:43, Greg Chicares wrote:
[...]
> GC> > Perhaps this class really wants delegating ctors: there's common
> GC> > post-initialization in realize_vector().
> GC> 
> GC> Vadim, tell me if you disagree, but I don't think that's a good idea
> GC> after all.
> 
>  I didn't comment on this part of the original 2017-01-26 email because I'm
> quite intimidated by the InputSequence class and I don't really understand
> what's going on with it

The header includes top-level documentation, but that presumes intimate
knowledge of the problem domain. OTOH, a problem-domain expert probably
wouldn't understand the documentation in the implementation, which speaks
of UDTs and ctors and uses a sort of pidgin BNF for maximum clarity. With
considerable effort, I've improved it at least to the point that I can
understand it (except for the parser, which I haven't gone over yet).

I'm not sure it's feasible to write inline documentation for any broader
audience. There's always
  http://www.nongnu.org/lmi/sequence_input.html
and the archived mailing-list discussions with Vaclav about the GUI.

If you have any specific questions, I'll try to answer them.

> GC> AIUI, delegation is most appropriate when there's one "complete" ctor
> GC> that initializes everything--then simpler ctors can delegate to it
> GC> (e.g., with some arguments defaulted) to make sure no initialization
> GC> step is skipped.
> 
>  Yes, absolutely.
> 
> GC> In this case, however, the InputSequence ctors take disjoint sets of
> GC> arguments. One takes only a single std::vector<double> argument with
> GC> one value for each year, and turns it into an InputSequence: it just
> GC> performs run-length encoding. Another takes numerous arguments that
> GC> represent the full context used in the GUI, but that one includes no
> GC> std::vector<double> argument. Therefore, any target ctor we might
> GC> write for this class would be a new, least-complete ctor consisting
> GC> only of this single line, which is the common initialization today:
> GC>     realize_vector();
> GC> and delegation would amount to nothing but elaborate syntactic sugar.
> 
>  If it could be useful to rewrite the "numerous arguments" ctor so that it
> would generate a vector (or two?), i.e. if this would make it simpler, then
> perhaps it could delegate to the other one, but in the current state I
> indeed don't see much point in using delegation here.

I think you're looking for the "canonical representation", alluded to in
one of the remaining "TODO" comments, which has not yet been defined.
That's the lingua franca, the least common denominator, that would tie
everything together: a lossless, run-length encoded representation of
the intervals_ member, i.e., what mathematical_representation() might
be except for its "TODO" comment. The number-vector and string-vector
representations are just projections we see on the wall of the crystal
cave, which lose some of the semantics: e.g.,
   1 1 1 0 0 0 0
might have meant
  (a) 1 for exactly three years, then 0
or
  (b) 1 until retirement, and 0 thereafter

OTOH, conversion from an InputSequence(vector) to the canonical
representation would not lose any information embodied in the vector
argument--we could arbitrarily deem (a) above, e.g., to represent the
"true" semantics. Then, having canonicalized the vector as the string
  "1 [0, 3); 0"
we could delegate to the many-argument ctor. That would parse the
canonicalized vector input, turning it back into a set of intervals
from which the original vector could be recovered, losslessly, but
with the overhead of instantiating a parser--so it wouldn't be a
simplification at all.

>  OTOH the current code still doesn't seem to be ideal to me because:
> 
> 1. It definitely looks like it ought to be possible to reuse the code
>    of the 3 ctors taking vectors in some way.

True. I hadn't thought of that, because, while the vector of doubles
is actually useful, I'm not sure whether we should keep the other two.

> 2. The "numerous arguments" ctor is IMO unwieldy and difficult to both
>    use and read. I would strongly consider using named arguments idiom
>    in order to avoid having a function with 9 parameters which is about
>    twice greater than optimal.

I'm not sure whether you mean boost::parameter or this:
  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4172.htm
: probably the former for now, and the latter if and when it is
standardized. But there are only a few combinations of defaulted
parameters that actually make sense. It took quite a lot of thought
to figure out this particular restriction:

  commit 7d6f0a101dac324c91f205501c2e237c2d47112c
  Assert preconditions for input-sequence default keyword

My present goal is to discover the restrictions implicit in the
implementation, which represent the design that has unconsciously
evolved. Until that implicit design is fully understood, the time
is not ripe for introducing more flexibility; and once it is
understood, I suspect we won't want any more flexibility.

The number of arguments per se doesn't bother me because I know
that the five 'int' arguments are a unitary, interdependent set
extracted from class Input, so notionally the arguments are
  std::string const& expression
  Input const&
and the three optional ones.

If OTOH the motivation is to make a change like this easier:

  commit 0482a6a9a8e78696b8bc1a2a60ccfcb0e140c329
  Remove default keyword from SequenceParser; reorder it in InputSequence

then yes, it would have had that effect, but that's the first time
since this class was introduced (probably in 2002) that the argument
list has changed, and I would very much rather make such simple
changes than introduce yet another dependency on yet another boost
library.

>  Last and really least: there are no [other] typos in this file, but this
> line:
> 
> // but these pardigms are disjoint, so it could not easily accomodate
> 
> manages to misspell both "paradigms" and "accommodate", somehow.

One teacher in my elementary school had a rule: whenever you misspelled
a word, you had to write it one hundred times. One day, I misspelled
"Iroquois", and wrote *Iriquois a hundred times. She thought I should
therefore write "Iroquois" one hundred squared times. My parents spoke
to the principal and had me removed from her class. That was in 1967
and since then, until now, I have had no occasion to write that word. I
think of that now because I discovered a decade ago that I had learned
an incorrect single-'m' spelling of "accommodate", and I've since made
a conscious effort to spell it correctly, so this alarmed me. However,
  $git log -G"accomodate" --patch
tells me it was introduced (in CVS) here:
  commit 73f326c348d063ae13dc93eb7ee77aee431857ea
  2005-01-14 19:47:45 +0000
  Initial checkin
Recently I changed the tense on that line (s/cannot/could not/), but
didn't notice the misspellings...so now I'm less alarmed.

> I hope
> that by noticing this I will have been able to have contributed at least
> something useful to this discussion.

Yes, and furthermore you've encouraged me to
  :setlocal spell spelllang=en_us
to mitigate the risk that I'd have to commit bdf4427 one hundred times.