On Sat, Jul 4, 2015 at 6:30 AM, Nick Wellnhofer <[email protected]> wrote:
> Lucifers,
>
> When reviewing the Clownfish Num classes, I made the following observations:
>
> - FloatNum_Compare_To isn't correct for large IntNums.
> - IntNum_Compare_To and Int64_Equals aren't correct for large
> FloatNums.
> - Num_Equals doesn't handle the corner case of IntNums near
> INT64_MAX correctly.
> - Converting FloatNums to IntNums doesn't check for overflow.
>
> This is fixed in my `num_compare` branch:
>
> https://github.com/nwellnhof/lucy-clownfish/commits/num_compare
This is definitely a nice improvement over what we have now,
so +1 to merge!
> Then there's an asymmetry when comparing BoolNums.
>
> BoolNum_Equals(CFISH_FALSE, Int32_new(0)) == false
> Int32_Equals(Int32_new(0), CFISH_FALSE) == true
>
> My original fix was to remove the specialization of BoolNum_Equals. But
> thinking more about it, it might be better to make BoolNum not inherit from
> Num at all.
+1
I like not inheriting from Num. And I agree that to eliminate the assymetry
of Equals, it makes sense to require explicit conversion to Boolean first.
> This would mean that a Boolean never equals a Num
As a consequence, that means Equals() will fail when we take a JSON-izable
data structure which includes a Boolean and round-trip it through a host which
does not offer a dedicated boolean type -- such as Perl.
Boolean -> IOK SV -> IntNum
I think that's for the best. We definitely are not goving to get
Equals-after-round-tripping-deserialized-JSON working across all hosts, so the
sooner we forswear that idiom and find alternatives, the better.
> and that
> Compare_To throws when comparing Booleans and Nums.
That seems acceptable -- it doesn't prevent us from evaluating the truthiness
of non-boolean numeric types (via To_Bool), it just stops us from using
Compare_To.
In retrospect, I regret the Num inheritance hierarchy. I wish that instead,
we had only Boolean, Integer, and Float, matching the types available in JSON.
That wouldn't solve all our problems, it would just give us a different set of
problems -- but I feel like that set of problems is more appropriate for
Clownfish to have.
* Num, IntNum, and FloatNum would all be eliminated, and Boolean, Integer,
and Float would all descend from Obj directly.
* Integer, Float, and Boolean would all live in their own files rather than
in Num.cfh/Num.c.
* Integer would have an internal `int64_t` value and FloatNum would have an
internal `double` value.
* Boolean, Integer, and Float would all be immutable.
* The Equals() and Compare_To() methods of Integer and Float would allow
cross-comparisons (using your new algorithms).
This wouldn't impact much because the box types for numerics are infrequently
encountered in Clownfish-flavored C.
Marvin Humphrey
