Re: [lmi] [lmi-commits] master 1a54e49 1/2: Clarify non-MEC solves

[Vadim Zeitlin]

On Tue, 15 Sep 2020 00:44:58 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> Someday maybe I'll attack this one:
GC> 
GC>     // TODO ?? This idiom seems too cute. And it can return -0.0 .
GC>     // Maximum repayment is total debt.
GC>     ActualLoan = -std::min(-RequestedLoan, RegLnBal + PrfLnBal);
GC> 
GC> although it's a low priority (in the problem domain, it rarely
GC> if ever makes sense to repay a loan against a life insurance
GC> policy).
GC> 
GC> This is the dark side of learning APL as a first language: I
GC> suppose that expression comes across to others as rather
GC> precious, but to me, it looks natural, and its only real
GC> defect is that it can return a negative zero...so I have to
GC> ask: how could it be written so that it's clear to everyone?
GC> 
GC> We can ignore the distinction between "Reg" and "Prf", and
GC> just consider the total "RegLnBal + PrfLnBal", which is the
GC> loan balance. "RequestedLoan" is just whatever was entered
GC> in the GUI: e.g., "1000" if you want to increase the loan
GC> balance by $1000, receiving $1000 in cash; or "-1000" if
GC> you want to pay back $1000 in cash and reduce the loan
GC> accordingly. The expression just means that you can't
GC> repay more than the total balance. How would you write that?
GC> Would
GC>   std::max(RequestedLoan, -(RegLnBal + PrfLnBal));
GC> be any clearer?

 It would, perhaps, be slightly more clear (IMHO max is generally easier to
understand than min and +max is definitely simpler than -min), but I still
wouldn't use this and would simplify the code further by:

1. Splitting 2 rather different cases.
2. Introducing intermediate variables with clear names.

 So ideally I'd write it like this:

        if(RequestedLoan < 0)
            {
            // Repayment can't exceed the outstanding balance.
            auto const TotalBalance = RegLnBal + PrfLnBal;
            auto RequestedRepayment = -RequestedLoan;
            if(RequestedRepayment > TotalBalance)
                {
                    RequestedRepayment = TotalBalance;
                }
            ActualLoan = -RequestedRepayment;
            }
        else
            {
            ActualLoan = RequestedLoan;
            }

which is, of course, much more verbose, but should also be painfully
obvious to anybody reading the code. Maybe there is some happy middle
ground between the lumbering procedural version and more concise but less
clear functional one, e.g. I think replacing the nested "if" with a call to
"std::min" would be fine, but IMHO the ideal version should be much closer
to the longer than the shorter one.

 Regards,
VZ

pgpqI7Fn9Qtep.pgp
Description: PGP signature