Re: [lmi] [lmi-commits] master b518132 4/4: Remove the latent defect just added

[Greg Chicares]

On 2018-09-06 01:04, Vadim Zeitlin wrote:
> On Wed,  5 Sep 2018 20:56:27 -0400 (EDT) Greg Chicares <address@hidden> wrote:
> 
> GC> branch: master
> GC> commit b518132022d0dab59226f584778973265b71e532
[...]
>  I've read the entire explanation, but this still looks weird to me. AFAICS
> all this was done just to avoid dividing by potentially 0 lines_per_group_
> (please correct me if I'm wrong).

Yes, that's exactly the goal. And yes, looking back at it a day later,
it is rococo, so I've reverted that last commit and used a different
technique instead in commit ad23b9ee.

> But if this is so, why not just use a
> usual safe_divide(x, y) function which throws/asserts if y == 0 for the
> initialization of groups_per_page_?

I've seen so many functions like
  safe_divide(x, y) {if(0 != y) return x / y; else return 1;}
(for various values of "1") in languages that don't have anything like
C++'s catchable exceptions, that I've developed a prejudice against
safe_divide() functions and thus didn't even consider that option.

Of course, you're instead saying
  T safe_divide(x, y) {if(0 != y) return x / y; else throw something;}
which is a horse of a different color; yet I share your preference
for the following alternative:

> Or, which I personally prefer, some
> safe_denominator(x) function which simply returns x if it's non-zero and
> throws if it's zero.

...because IMO the problem is not that we attempted a division and it
failed--rather, it is that we used some variable as a denominator but
we failed to ensure that it fulfills a denominator's contract; thus:

  T safe_denominator(x) {if(0 != x) return x; else throw something;}

  foo::foo(T some_argument, T another_argument)
    :denominator {safe_denominator(some_argument)}
    ,ratio {another_argument / denominator}

And I plan to grow the ctor-initializer:

    ,quotient    {b / a}
    ,remainder   {b % a}

and would prefer not to write both safe_divide() and safe_remainder()
when all that's needed is one safe denominator.

> This would seem to be much simpler and avoid the need
> for a clumsy (IMHO) ctor_args_are_sane_ field.

Yes, thanks for challenging that. I hope you'll find commit ad23b9ee
less obtrusive.

> It is also arguably safer
> because if you always use safe_xxx() when dividing integers, you don't have
> to check that you had checked preconditions at some previous moment.

I would hesitate to provide a generally-available utility function
for that purpose because it's hard to track down where an exception
was thrown when a messagebox just says "Cannot divide by zero".
Addressing the general case would require a macro--either
  LMI_ASSERT_NONZERO(denominator);
  ratio = numerator / denominator;
or, much better than writing a new macro, just
  LMI_ASSERT(0 != denominator);
  ratio = numerator / denominator;
except that that's the original problem.