Re: [lmi] wx_test_expiry_dates.cpp

[Vadim Zeitlin]

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?

GC> Requiring a date on the command line makes this facility harder to use.
GC> But that's okay, and even desirable: we want it to be hard to misuse.
GC> We want its use to be possible only as a deliberate, mindful act. So let's
GC> do it your way, but flatter my vanity with s/check-date/prospicience/, okay?

 The attached patch is supposed to be flattering enough as it uses
prospicience both for the name of the command line option and also for the
name of the new global_instance field and methods, but please let me know
if you see any other places where I failed to use it.

 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?

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.

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.

 Thanks in advance!
VZ

0001-Add-a-new-prospicience-command-line-option.patch
Description: Text document