Re: [lmi] Calculation Summary columns selection

[Greg Chicares]

On 2006-11-6 16:12 UTC, Evgeniy Tarassov wrote:
> This is what i was going to do to implement the feature. Could you
> please comment on it if you see any problem or maybe a better way to
> accomplish the same thing, as i'm not sure that it is the best one
> could do using the existing lmi code.

I should have explained more clearly that it's okay to take extreme
shortcuts here. All we need is the simplest thing that works today.
Code adapted from one of the wx sample programs would work just fine:
maybe 'samples/xrc/derivdlg.cpp'. I'm thinking of the kind of thing
you guys could write in your sleep.

The long-term solution is to provide a configurable-settings editor
using the MVC framework. That shouldn't require much work, but it
does require a deep understanding of that framework--which does far
more than we need here today. For instance, it validates numbers,
changes available selections in the context of other inputs, and
enables or disables input fields; but here, we have twelve identical
comboboxes that are always enabled and always contain the same
selections.

For the short term, wouldn't it be much easier to add a wxDialog that
looks the same as the supplemental-report tab (without its checkbox)?
All we need is a dialog to select up to twelve strings from a static
set. The set of strings is given by macro 'report_column_NAMES' in
'mc_enum_types.xpp' (omit the first element, '"[none]"'). The dialog
can use the wxxrc resource 'reports_panel' in 'xml_notebook.xrc'
(with the checkbox omitted). Then all we need is a menuitem to pop up
the dialog, and code to transfer the selections between the dialog
and a new entity in 'configurable_settings.xml'. And 'OK' and 'Cancel'
buttons.

You could even hardcode the strings in the '.xrc' file:

  <content>
      <item>AttainedAge</item>
      <item>PolicyYear</item>
      ...
  </content>

That's good enough for now--whatever is easiest. Don't worry about
updating the strings automatically when more columns become available
in the future. Take hardcoded shortcuts wherever possible. We'll
refactor everything before this becomes a problem; we just don't have
time to do that now.

This doesn't need to be elegant. It'll be replaced by the long-term
solution soon enough.

> step 0) code duplication
> 
> Implement a PropertiesModel class which inherits from MvcModel. This
> class would have the same 12 columns list as in Input class.
> class PropertiesModel
>    :public MvcModel
> {
>    mce_report_column CalculationSummaryColumn00;
>    ...
>    mce_report_column CalculationSummaryColumn11;
> };

I think it'd be far easier to avoid the 'mc*' classes, which are
overkill for this task. I'd suggest

  std::string CalculationSummaryColumn00;
  ...
  std::string CalculationSummaryColumn12;

in a class derived from wxDialog. There's no need to inherit from
class MvcModel for this.

> Edit xml_notebook.xrc and duplicate the xml code describing columns
> layout (as for Supplemental Report columns).

Agreed.

> Write a code that reads/writes value from/to configurable_settings.

Agreed.

> Add set_XXX methods to configurable_settings (or derive from
> configurable_settings and make settings editable through the derived
> class).

Isn't it easier to store the selections as a single text node,
with substrings delimited by whitespace?

'configurable_settings.xml':
  <calculation_summary_columns>
    some_column_name
    some_other_column_name
    ...
  </calculation_summary_columns>

'configurable_settings.cpp':

  class configurable_settings {
    public:
      std::string calculation_summary_columns_;

Public data is not elegant. But this doesn't have to be elegant.
It's just temporary. It'll be replaced soon enough.

> Add a method to write the changes in configurable_settings back into
> configurable_settings.xml file.

Agreed.

> step 1) code refactoring
> 
> Add a common base class for the PropertiesModel and Input. Which would
> contain the implementation of the pure virtual methods from MvcModel
> class and would implement it through the MemberSymbolTable class
> methods.

The MemberSymbolTable stuff in 'configurable_settings.cpp' makes
it easy to address all entities: the ctor has code like this:

    typedef xml_lmi::ElementContainer::const_iterator eci;
    for(eci i = elements.begin(); i != elements.end(); ++i)
        {
        operator[]((*i)->get_name()) = xml_lmi::get_content(**i);
        }

to get each one from the xml file, and you can just reverse that
to set each one. It's just a "properties list". I think that'll
save you time, and there's similar code in other modules.

I don't think MemberSymbolTable has to be used in the new dialog,
which only needs to represent a single whitespace-delimited string
as up to twelve strings, each in a combobox.

> Implement a base type for Input::SupplementalReportColumnXX and
> PropertiesModel::CalculationSummaryColumnXX that will read its domain
> values from format.xml file. The existing type is mce_report_column
> which has its values hardcoded in mc_enum_types.xpp.

Here, std::string should be good enough for now.

> step 2) code changes
> 
> I will try to change Mvc code to support complex data types such as
> ordered vectors, to avoid using magic numbers like 12 for column
> number.

You're quite correct: twelve is a magic number, and in the long term
we want to avoid hardcoded limits like that. But this short-term
solution can be inelegant. We'll trade elegance for expediency now,
and then replace it completely later.

> Do you think that this mini-roadmap is good?
> The step 0 is really the minimal thing to do to implement the
> requested feature but basicly its a copy/paste from the existing code.

I think it can be even easier if you just use std::string and
class wxDialog.

> Step 1 is about code refactoring,

I understand, but this is a temporary thing, and duplication is okay
for now: we'll remove it later.

> but still the magic list of 12
> columns is far from ideal.

Yes. For today, we don't need to achieve the ideal.

> And the step 3 is the most difficult as it
> involves modifications in the existing Mvc code as far as i could
> understand the code.

Someday we can discuss that. It could be a useful extension.
For today, though, we haven't time to redesign the MVC stuff.