Re: [lmi] wx_test_calculation_summary.cpp

[Greg Chicares]

On 2015-03-06 19:14, Vadim Zeitlin wrote:
> On Sun, 25 Jan 2015 01:01:06 +0000 Greg Chicares <address@hidden> wrote:
> 
> [Actually, not quite, I surreptitiously updated the quoted text below to
>  correspond to the later version of the specification, i.e. incorporated
>  the changes to it from the next day, 2015-01-26]
> 
> GC> /// Test calculation summary.
> GC> ///
> GC> /// Iff the '--distribution' option is specified, then:
> GC> ///   File | Preferences
> GC> /// make sure that "Use built-in calculation summary" is checked, and
> GC> /// that the saved selections (those that would become active if the
> GC> /// checkbox were unchecked) exactly match the default selections given
> GC> /// by default_calculation_summary_columns().
> 
>  I have a question about default_calculation_summary_columns(): this is a
> private function in configurable_settings.cpp which is inaccessible to the
> testing code. Does its mention here mean that I have the licence to make it
> public and use it? Or should I try, as usual, to make the test work without
> any changes to the main program code?

Can we do it with friendship? E.g.:

 class LMI_SO configurable_settings
     :        private lmi::uncopyable   <configurable_settings>
     ,virtual private obstruct_slicing  <configurable_settings>
     ,        public  xml_serializable  <configurable_settings>
     ,        public  MemberSymbolTable <configurable_settings>
 {
+    friend class wx_test_case_configurable_settings;
+
   public:
     static configurable_settings& instance();

>  In the latter case, I see two possibilities:
> 
> (a) Just duplicate the columns from default_calculation_summary_columns()
>     in the test code. This is the simplest solution but it has the obvious
>     shortcoming of duplicating the same data in two places with the usual
>     risk of the two copies getting out of sync inherent to it.

Effective, but crude: a last resort.

> (b) Use effective_calculation_summary_columns() to access the default
>     columns via a "backdoor": while this function is not documented to
>     do it, it currently forwards to default_calculation_summary_columns()
>     if use_builtin_calculation_summary() is false. I think this is
>     preferable to (a) but I'm not sure how robust is this and whether
>     effective_calculation_summary_columns() is guaranteed to continue to
>     work as it does now in the future.

I think it's pretty robust: I can't see any reason for ever changing
its behavior.

So my preference order is:
  best: friendship
  next best: (b) [sneak through existing back door]
  second worst: (a) [dubious and fragile code duplication]
  worst: break encapsulation with s/private:/public:/ [FORTRAN, not OOP]

> GC> /// File | Preferences
> GC> /// uncheck "Use built-in calculation summary"
> GC> /// set all "Column" controls to "[none]"
> GC> ///   do this by simulating {Tab Home} repeatedly because that is
> GC> ///   much faster and is what a smart user does
> 
>  In principle, there is no guarantee that doing this would work on all
> platforms, e.g. it doesn't work under OS X by default.

I'm just curious, because OS X is often cited as a paragon of usability:
does that OS force you to step through all choices when you already know
which one you want? But see below before answering this, as I might just
be misunderstanding what I think I'm seeing.

> Of course, lmi
> doesn't target that platform currently anyhow, but it still seems a bit
> unwise to commit to some particular way of selecting items. The way it's
> done currently, i.e. using wxUIActionSimulator::Select(), is safer in the
> sense that if this doesn't work for some new platform or a new version of
> the existing one, it would be fixed in a newer version of wxWidgets itself
> without requiring any changes to lmi code.

Oh. I didn't read the code in detail while writing the specifications,
so I didn't realize that wxUIActionSimulator::Select() was used here.

>  Please let me know if this argument seems convincing to you and, if so,
> whether I should update the specification to remove the last two lines
> (otherwise I'd need to update the code to avoid the use of Select()).

Instead of these two prescriptive lines:
> GC> ///   do this by simulating {Tab Home} repeatedly because that is
> GC> ///   much faster and is what a smart user does
let me present my thoughts descriptively.

Watching this test run, I formed an impression that the code was stepping
through each combobox item, as if it did this:
    loop0: select previous or next item
    if (it's the one we want) then goto done
    else wait a moment, then goto loop 0
    done:
I say this because it seems...to...do...that...ever...so...slowly and
it becomes tiresome to watch--I want to shout "Hit Home! Jump to [none]!".

If Select() really does do that, then I'd be glad to see it optimized.
If it doesn't, and my impression is mistaken, then I'd like it to be
corrected. Does my perception of the behavior make any sense to you?