Re: [lmi] Currency class

[Vadim Zeitlin]

On Thu, 17 Sep 2020 00:41:52 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

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

 Yes, indeed, and viewing ef7fe2fc3 (Change type of currency variables to
'currency' in headers, 2020-08-20) with "-w --color-words='[A-Za-z0-9]+'"
options shows that 99.9% of the changes are s/double/currency/ and the
addition of "currency.hpp" inclusions (the remaining one being the addition
of "// Antediluvian." comment). The next commit, d137177b1 (Currency,
2020-09-09), however already has some less trivial changes, so I'll need to
take more time to go over them...

GC> Adding explicit conversions with currency(double) and currency::d()
GC> is ugly because it's in your face everywhere. The only thing uglier
GC> is implicit conversions that hide everything, because what we cannot
GC> see we cannot improve. A favorite example is [master]:
GC> 
GC>     double max_loan =
GC>           AVGenAcct + AVSepAcct * MaxLoanAVMult
GC>         + AVRegLn + AVPrfLn
GC>         - anticipated_deduction(MaxLoanDed_)
GC>         - std::max(currency(), SurrChg())
GC>         ;

 Just because I was worried about this: I checked, and there are
parentheses around "AVGenAcct + AVSepAcct" in master, so the change below

GC> which looks as innocent as Dorian Gray (although most or all of those
GC> MixedCaseVariables are of type currency), so I changed that to [branch]:
GC> 
GC>     double max_loan =
GC>           (AVGenAcct + AVSepAcct).d() * MaxLoanAVMult
GC>         + (AVRegLn + AVPrfLn).d()
GC>         - anticipated_deduction(MaxLoanDed_).d()
GC>         - std::max(currency(), SurrChg()).d()
GC>         ;

doesn't change the value of the expression, as I was afraid it did. I.e.
there is only a typo in the email message, but not in the code.

GC> which is as stomach-turning as his portrait, but that's what really
GC> occurs when promiscuous implicit conversions abound. Surely this can
GC> be improved by rewriting the expression--perhaps
GC> 
GC>     currency max_loan =
GC>           round_somehow.c(AVGenAcct + AVSepAcct) * MaxLoanAVMult)

 Another parenthetical aside: I believe there is a missing opening
parenthesis here (and not an extra closing one), i.e. round_somehow()
should apply to the whole expression.

GC>         + AVRegLn + AVPrfLn
GC>         - anticipated_deduction(MaxLoanDed_)
GC>         - std::max(currency(), SurrChg())
GC>         ;
GC> 
GC> but the problem is that the exact type of each variable must be
GC> considered carefully to avoid introducing errors that will cost
GC> far more to find and fix than to avoid in the first place. And
GC> that's just one expression, out of how many expressions in these
GC> thousands of lines of code?

 Is there any kind of rule that could be associated with MaxLoanAVMult to
indicate how should the result of multiplying by it be rounded? If there
were, we could probably do something smart about doing it automatically.
But if the results of multiplying by the same factor needs to be rounded
differently depending on where it occurs, then I indeed don't see any other
choice than reviewing all such expressions individually.

GC> > Also, if you have any pointers to the parts it might
GC> > be especially worth to concentrate on, they would be very welcome.
GC> 
GC> The only thing that's almost clever is the intrusion of a c()
GC> function into class template round_to. All else is tedium.

 Thanks, I'll make sure not to miss this one.

GC> But here's my dilemma. In the latest README.branch file, I use
GC> objective data to make a case that using a currency class must
GC> entail a fearsome performance penalty. Yet OTOH that simply
GC> cannot be.

 This is indeed surprising, but I've vowed to never say "it cannot be"
about any benchmarking results after seeing too many "impossible"
measurements. If it does happen, all I can -- perfectly unhelpfully -- say
is that it clearly can be. But there is still an interesting, and
potentially helpful, question to answer, which is "how does it come to
pass?". So, if this is the more or less final version of your code, I'd
like to profile the code and see if I can discover where does this slowdown
come from. Should I/we do this? If so, could you please explain how exactly
do you obtain these measurements?

 Thanks in advance,
VZ

pgp5vTvALxUtC.pgp
Description: PGP signature