Re: [lmi] Currency class

[Greg Chicares]

On 2020-09-16 21:38, Vadim Zeitlin wrote:
> On Wed, 16 Sep 2020 21:00:42 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> I've pushed a new valyuta/002 9433c8e2 to origin, and
> GC> would appreciate your comments. There's a summary at the
> GC> top of 'README.branch.patch'.
> 
>  Thanks, I'll try to look at it a.s.a.p., but seeing
> 
>       27 files changed, 1148 insertions(+), 2033 deletions(-)
> 
> is a bit intimidating. It looks like there are many "trivial" changes here
> (i.e. replacements of "double" with "currency" and the extra ".d()"), but
> it still will take some time for me to digest all this and I just wanted to
> warn you about this.

Replacing "double" with "currency" in declarations (headers, e.g.)
is the sine qua non, of course. There are many such replacements,
because classes BasicValues and AccountValue have many variables
that yearn to be currency.

Adding explicit conversions with currency(double) and currency::d()
is ugly because it's in your face everywhere. The only thing uglier
is implicit conversions that hide everything, because what we cannot
see we cannot improve. A favorite example is [master]:

    double max_loan =
          AVGenAcct + AVSepAcct * MaxLoanAVMult
        + AVRegLn + AVPrfLn
        - anticipated_deduction(MaxLoanDed_)
        - std::max(currency(), SurrChg())
        ;

which looks as innocent as Dorian Gray (although most or all of those
MixedCaseVariables are of type currency), so I changed that to [branch]:

    double max_loan =
          (AVGenAcct + AVSepAcct).d() * MaxLoanAVMult
        + (AVRegLn + AVPrfLn).d()
        - anticipated_deduction(MaxLoanDed_).d()
        - std::max(currency(), SurrChg()).d()
        ;

which is as stomach-turning as his portrait, but that's what really
occurs when promiscuous implicit conversions abound. Surely this can
be improved by rewriting the expression--perhaps

    currency max_loan =
          round_somehow.c(AVGenAcct + AVSepAcct) * MaxLoanAVMult)
        + AVRegLn + AVPrfLn
        - anticipated_deduction(MaxLoanDed_)
        - std::max(currency(), SurrChg())
        ;

but the problem is that the exact type of each variable must be
considered carefully to avoid introducing errors that will cost
far more to find and fix than to avoid in the first place. And
that's just one expression, out of how many expressions in these
thousands of lines of code?

> Also, if you have any pointers to the parts it might
> be especially worth to concentrate on, they would be very welcome.

The only thing that's almost clever is the intrusion of a c()
function into class template round_to. All else is tedium.

But here's my dilemma. In the latest README.branch file, I use
objective data to make a case that using a currency class must
entail a fearsome performance penalty. Yet OTOH that simply
cannot be. Thought experiment: suppose lmi had always expressed
"currency" amounts in cents, rather than in dollars and hundredths,
and suppose the datatype was still 'double' (but scaled). Then
it would have the same performance as it does in production today;
its values would just be one hundred times larger, and would have
to be scaled for printing, but such scaling couldn't make it more
than about one percent slower. So which conclusion is right--the
first one, which I can support with data (and I can't seem to
produce data that refute it)...or the "OTOH" second, which
obviously has to be true?