Re: [lmi] wx_test_expiry_dates.cpp

[Greg Chicares]

On 2014-12-14 23:22, Vadim Zeitlin wrote:
> On Sun, 14 Dec 2014 16:53:46 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> and I think you'd accept this renaming
> GC>   --prospicience=YYYYMMDD
> GC> because I'm now very attached to that word.
> 
>  While working on making this patch, I've finally realized why: it's
> probably because it brings a degree of built-in security due to being so
> easy to misspell, isn't it?

Exactly. For a native speaker at least, it's well-nigh impossible to spell:
we want to insert an 's' by analogy with "omniscience" or "conscience".
We can spell "deficience" and "sufficience" and "proficience", but those
are words that we don't actually use, preferring variants that replace the
terminal 'e' with 'y'. And "prospicience" violates this general rule, too:
  http://en.wikipedia.org/wiki/I_before_E_except_after_C

It's even harder to spell than "idiosyncrasy" for people who work in a
bureaucracy and live in a democracy.

>  I have a couple of other, potentially more serious even though not as
> linguistically interesting, questions concerning this patch:
> 
> 1. I decided to make --prospicience another hidden option by mapping it to
>    an unprintable short option character. I don't know if this is correct
>    however, perhaps it is meant to be public and so appear in --help
>    output for example?

Perfect. Thanks for thinking of that.

> 2. I used wxDateTime for parsing the YYYYMMDD string passed on the command
>    line because doing it with istream is just too painful: I started
>    writing the code to do it but couldn't bring myself to finish it when
>    I knew it could be done with a single wxDateTime::ParseFormat() call. I
>    am not sure, however, how do you feel about using wxDateTime instead of/
>    in addition to calendar_date in lmi code, even in its wx-specific part,
>    so if you'd prefer to do it without using wxDateTime, please let me know.
>    But in this case I'd like to suggest adding a (possibly static) method
>    for parsing YYYYMMDD strings to calendar_date itself as it could
>    possibly be useful elsewhere and I'd rather put it where it logically
>    belongs instead of keeping this in the command line parsing code.

Could you simply adapt this example from 'calendar_date_test.cpp'?

    // Construct from ymd_t.
    calendar_date date4(ymd_t(18991231));
    BOOST_TEST_EQUAL(dublin_epoch, date4);

An istream can read "18991231" as the integer 18,991,231 , which is
then converted via 'ymd_t'.

> 3. While I've tested this patch with and without --ash_nazg and
>    --distribution options as well as with different valid and invalid
>    values of --prospicience option itself and everything seemed to behave
>    as expected, it is still slightly unnerving that there are no unit tests
>    for this. Unfortunately I didn't see any way to test this easily, so I
>    didn't do anything about it, please let me know if I missed anything or
>    if you'd like me to find some way of testing this even if it can't be
>    done easily.

I'll see what Kim has to say when she returns; meanwhile:
committed 20141215T1454Z, revision 6079.