BigInteger should extend Number

[sergeych]

as java's and Kotlin's ones do. It is expected and it causes (from my experience) strange bugs as the replaced BigInteger is not recognized by existing software as a Number.

[ionspin]

Hi @sergeych, can you give me a bit more details on the strange bugs that you encountered?

[sergeych]

For example, we widely use a library that checks the argument as if( obj is Number ) process(obj.toDouble()) - it is an approach that at least in our companies is very popular. For example, in the core of our blockchain project we check contract's data field to be a Number, also in one very old (2007) binary serialization library (that actually is widely used) there is a code that does not work being recompiled with your great library, it is somewhat like:

 if (obj instanceof Number) {
                Number n = (Number) obj;
                if ((obj instanceof Integer) || (obj instanceof Long)) {
                    long value = n.longValue();
                    if (value >= 0)
                        writeHeader(TYPE_INT, value);
                    else
                        writeHeader(TYPE_NINT, -value);
                    return this;
                }
                if (obj instanceof BigInteger) {
                    BigInteger bi = (BigInteger) obj;
                    if (bi.signum() >= 0)
                        writeHeader(TYPE_INT, bi);
                    else
                        writeHeader(TYPE_NINT, bi.negate());
                    return this;
                }
                if(obj instanceof BigDecimal) {
                    String s = obj.toString();
                    return writeString(s);
                }

The problem is, the BigDecimal processing is packed inside (is Number) check. So, it does not work out of the box. I suppose there are more places like this if your great library would be widely used.

[ionspin]

I understand, unfortunately I don't think implementing Number would lead to a good API, as it is just narrowing operations, and could be misleading when used. Since the API is not yet stabilized I'll keep this issue open and have some more time to think about it.

The reasoning why I don't want to implement Number is that I think it would be misleading to quietly narrow from a i.e. 128 bit integer to a 32 bit integer. While currently the library does that because the default value exactRequired of narrowing functions is set to false, I'm inclined to change that to true and ask for a specific permissions from user when losing information because of narrowing.

For completeness here is the Number.kt interface that is being discussed:

/**
 * Superclass for all platform classes representing numeric values.
 */
public abstract class Number {
    /**
     * Returns the value of this number as a [Double], which may involve rounding.
     */
    public abstract fun toDouble(): Double

    /**
     * Returns the value of this number as a [Float], which may involve rounding.
     */
    public abstract fun toFloat(): Float

    /**
     * Returns the value of this number as a [Long], which may involve rounding or truncation.
     */
    public abstract fun toLong(): Long

    /**
     * Returns the value of this number as an [Int], which may involve rounding or truncation.
     */
    public abstract fun toInt(): Int

    /**
     * Returns the [Char] with the numeric value equal to this number, truncated to 16 bits if appropriate.
     */
//    @Deprecated("Direct conversion to Char is deprecated. Use toInt().toChar() or Char constructor instead.", ReplaceWith("this.toInt().toChar()"))
    public abstract fun toChar(): Char

    /**
     * Returns the value of this number as a [Short], which may involve rounding or truncation.
     */
    public abstract fun toShort(): Short

    /**
     * Returns the value of this number as a [Byte], which may involve rounding or truncation.
     */
    public abstract fun toByte(): Byte
}

In any case thanks for bringing this issue to light!

[ionspin]

I am closing this as it has been proven several times that narrowing by default causes issues where people are unaware of precision loss. The default behavior now is throw an exception on precision loss when narrowing, and that behavior prevents implementing Number interface which would allow precision loss silently.

[elect86]

that behavior prevents implementing Number interface which would allow precision loss silently

Sorry but that is false, simply wire toX with exactRequired = true and then you get no silent precision loss

[ionspin]

Consumers of java Number interface wouldn't expect any of the conversion method to ever fail, and implementing your proposal would make it fail (keep in mind even converting BigDecimal value 0.1 to IEEE754 float results in a precision loss), so this would still make for a lousy API.

[elect86]

If you don't want silent precision loss that's the only logical way.

You may describe the problem in the exception in order to make them aware.

If they want floating points they have to go though an integer first. That's nothing unusual for people dealing with Number nowadays, since toChar() is going throw the same transformation (toChar() is deprecated and substituted with toInt().toChar())

Moreover, Jvm does offer solutions to take care of exceptions

Simply not extending it because, under some conditions, some code may fail is wrong, imho

but this is your lib and that's my 2c

nonetheless, great lib

[ionspin]

nonetheless, great lib

Thanks!

If they want floating points they have to go throw an integer first.

I never heard of this, how do you get a floating point without precision loss by going through integer?

[elect86]

I never heard of this, how do you get a floating point without precision loss by going through integer?

Sorry, I meant though instead of throw

Let me explain, the precision loss is actually there, you just try to make it explicit to the user by forcing him to convert the BigInteger to Int (or Long) and then to the floating type

This means toFloat() and toDouble() shall, obviously, throw exceptions

[ionspin]

I see, thanks for clearing up. In any case I stand by my decision not to extend Numbers. Thanks for the discussion it is really appreciated!