Re: [lmi] Code review: product editor

[Greg Chicares]

On 2007-3-12 17:26 UTC, Vadim Zeitlin wrote:
> On Mon, 12 Mar 2007 17:19:44 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Why don't we just write two separate typesafe class templates:
> GC>   template<typename ValueType> UnidimensionalTable;   // MultiDimTable1
> GC>   template<typename ValueType> MultidimensionalTable; // MultiDimTable7
> GC> without any metaprogramming?
> 
>  The main reason is that it would be impossible to reuse the code to
> implement another table. I.e. everything will be completely hardcoded. And

Yes, the axes would be completely hardcoded. They're inherent in the
problem domain. Life-insurance rate tables vary by few axes:
  // gender
  // underwriting class
  // smoker class
  // issue age
  // underwriting basis
  // state of jurisdiction
  ...and duration since issue
The last one that was added to that list is 'smoker class'; that happened
in the 1970s. We may add others, but we'll add them for all entities.
That costs little, because most entities actually vary across only one or
two axes, and the way we store and retrieve data takes advantage of that
sparsity.

The only tables I can think of that aren't accommodated by that paradigm
are paired data, like our 'tiered' entities; it's been good enough so far
not to let such data vary by any of the seven axes above, and I don't hear
any user asking to change that.

> another reason is that there will be some code duplication between
> UnidimensionalTable and MultidimensionalTable as they won't be able to
> just use the same generic class any more then.

Yes, there will indeed be some code duplication, but not very much.
Let's measure it (you can look at the footnote instead of
generating this yourself [1]).

First:
  make CPPFLAGS='-E' check_physical_closure >/dev/null 2>&1

Then patch 'multidimgrid_safe.hpp' (I don't know an easier way):
+ int XXXXXXXX = 14;
  /// real code declaring MultiDimGridN classes
  BOOST_PP_REPEAT_FROM_TO(1, MAX_MULTIDIMGRID_AXIS, MDTABLE_DECLARE_FOR_, ~)
+ int YYYYYYYY = 14;

Then:
  make CPPFLAGS='-E' check_physical_closure >../log 2>&1
  <../log sed -e'/XXXX/,/YYYY/!d' -e's/;/;\n/g' >eraseme
  wc eraseme
    204    1430   10631 eraseme

Two hundred lines for eight expansions. The two expansions we want
total about forty lines. We'd add whitespace, which might double that,
but we'll lose about thirty lines by removing the macro, so this might
cost on the order of fifty extra lines of code. I would find those two
similar classes easier to read and maintain than one metaprogram that
generates them.

> GC> But we don't need to make the axes generic.
[...]
> GC> We can hardcode the axes.
> 
>  I see. It looks we really did complicate the code too much here then,
> just as I was afraid. I still feel slightly sad breaking what looked
> like nice and generic interface

Yet we can always retrieve a metaprogram from the cvs attic if we need it.

> (which might be potentially useful for
> display of other data and not only in the product editor BTW)

Where else but in the product editor? Do you have a concrete
example in mind?

> and replacing
> it with the hard coded axis set but if you're absolutely sure that we're
> never going to need these classes for anything but the product editor, so
> be it.
> 
>  Could you just please confirm it for the very last time?

Confirmed. If we ever find a need for axes to be generic, we'll
look in the attic. I can't absolutely guarantee that'll never
happen, but I'm satisfied that it's implausible.

----------

[1] Macro expansions for MultiDimTable1 and MultiDimTable7:

template <typename T, typename V0> class MultiDimTable1 :public MultiDimTableAny
{ public: typedef T ValueType;
 static unsigned int const AxisNumber = 1;
 typedef V0 AxisValueType0;
 virtual T GetValue( V0 v0) const = 0;
 virtual void SetValue ( V0 v0 ,T const& value ) = 0;
 virtual unsigned int GetDimension() const;
 virtual MultiDimAxis<V0>* GetAxis0() = 0;
 virtual MultiDimAxisAny* DoGetAxisAny(unsigned int nn);
 protected: virtual boost::any DoGetValue(Coords const& coords) const;
 virtual void DoSetValue(Coords const& coords, boost::any const& value);
 private: MultiDimTableTypeTraits<T> converter_;
 virtual std::string ValueToString(boost::any const& value) const;
 virtual boost::any StringToValue(std::string const& str) const;
};
template <typename T> class MultiDimTable7 :public MultiDimTableAny
{ public: typedef T ValueType;
 static unsigned int const AxisNumber = 7;
 typedef V0 AxisValueType0;
 typedef V1 AxisValueType1;
 typedef V2 AxisValueType2;
 typedef V3 AxisValueType3;
 typedef V4 AxisValueType4;
 typedef V5 AxisValueType5;
 typedef V6 AxisValueType6;
 virtual T GetValue( V0 v0 , V1 v1 , V2 v2 , V3 v3 , V4 v4 , V5 v5 , V6 v6) 
const = 0;
 virtual void SetValue ( V0 v0 , V1 v1 , V2 v2 , V3 v3 , V4 v4 , V5 v5 , V6 v6 
,T const& value ) = 0;
 virtual unsigned int GetDimension() const;
 virtual MultiDimAxis<V0>* GetAxis0() = 0;
 virtual MultiDimAxis<V1>* GetAxis1() = 0;
 virtual MultiDimAxis<V2>* GetAxis2() = 0;
 virtual MultiDimAxis<V3>* GetAxis3() = 0;
 virtual MultiDimAxis<V4>* GetAxis4() = 0;
 virtual MultiDimAxis<V5>* GetAxis5() = 0;
 virtual MultiDimAxis<V6>* GetAxis6() = 0;
 virtual MultiDimAxisAny* DoGetAxisAny(unsigned int nn);
 protected: virtual boost::any DoGetValue(Coords const& coords) const;
 virtual void DoSetValue(Coords const& coords, boost::any const& value);
 private: MultiDimTableTypeTraits<T> converter_;
 virtual std::string ValueToString(boost::any const& value) const;
 virtual boost::any StringToValue(std::string const& str) const;
 };