TINKERPOP-3166 Implement asNumber() step

[xiazcy]

Finishing up the asNumber() step implementation following #3136.

This allows users to cast numbers into desired types, as well as to parse numerical string into desired numbers.

VOTE +1

[andreachild]

I would expect the basic happy path example to be included, something like g.inject("1234").asNumber().

[lyndonbauto]

This is cool.

Is there a way this could be used in this format:

g.V(id).outE().values("count").sideEffect(__.count().as("cnt")).order().by("count", Order.desc).limit(__.math("cnt * 0.1").asNumber())

somewhere down the road?

(This query might be a poor way to do this, I made it off the top of my head, really just wondering if this could be used in a way where someone could want to grab the top % of something and not have to count in 1 query then use that count and the percent to limit in the second)

[spmallette]

nit: "step" not "steps"

[spmallette]

new steps like this should probably come with an addition to the semantics doc.

[spmallette]

confused by this type declaration of DT and the use of 199. is this meant for N? and isn't the next number 205, not 199? and curious why serialization tests would be passing under this configuration....

[xiazcy]

Oh this looks like a miss from copying. And yea, I wonder why tests passed.

[spmallette]

Radical idea at this point but does N need to exist at all? It really doesn't serve any functional purpose that simply referencing the Number class would not already do. would it be more clean to only define "N" where necessary? Like, the Grammar would need an N to understand int, float, etc. and Javascript would need a way to reference BigDecimal or whatever, but does Java (or .NET) itself need a redefinition of number types?

[Cole-Greer]

Is there loss of precision when directly using Float.doubleValue()?

[xiazcy]

By "loss" I mean Float.doubleValue() itself will add arbitrary decimal places when widening to double, since it's not an exact representation. e.g. "1.23" to float 1.23 becomes 1.2300000190734863.

[Cole-Greer]

We should also use BigDecimal.longValueExact() as BigDecimal.longValue() may result in some big numbers appearing small:

new BigDecimal(Long.MAX_VALUE+"1").longValue()
==>-9

gremlin> g.inject(new BigDecimal(Long.MAX_VALUE+"1")).asNumber(N.nshort)
==>-9

[Cole-Greer]

There's a strange inconsistency that arises by using long as the intermediate form for all of these integer types.

gremlin> g.inject(new Double(Integer.MAX_VALUE+"1")).asNumber(N.nint)
Can't convert number of type Double to Integer due to overflow.
Type ':help' or ':h' for help.

gremlin> g.inject(new Double(Long.MAX_VALUE+"1")).asNumber(N.nlong)
==>9223372036854775807

Double.longValue() does a primitive cast from double to long, which implicitly binds out-of-range values to Long.MAX_VALUE or Long.MIN_VALUE. Perhaps an extra check is needed in getLong() to throw an exception if the value exceeds the range of a long. The same concern would apply to float-to-long conversions.

[andreachild]

There's an existing coerceTo method which has similar casting logic (but instead of throwing on overflow, it will return the original type). I am wondering if the logic between coerceTo and castTo should be made consistent (can castTo delegate most of the logic to coerceTo?) and if it's a problem if they are inconsistent.

[andreachild]

BigInteger also needs to be handled. Can add this unit test:

@Test(expected = ArithmeticException.class)
public void shouldOverflowIfBigDecimalCannotFitInFloat() {
    NumberHelper.castTo(new BigDecimal("1E400"), N.float_);
}

@Test(expected = ArithmeticException.class)
public void shouldOverflowIfBigIntegerCannotFitInFloat() {
    NumberHelper.castTo(new BigInteger("1").shiftLeft(2000), N.float_);
}

[andreachild]

Suggested change

	// BigDecimal to double will overflow into Infinity, we want to handle this
	// BigDecimal or BigInteger to double can overflow into Infinity, we want to prevent this

[andreachild]

Also need to handle BigInteger. Can add these unit tests:

@Test(expected = ArithmeticException.class)
public void shouldOverflowIfBigDecimalCannotFitInDouble() {
    NumberHelper.castTo(new BigDecimal("1E400"), N.double_);
}

@Test(expected = ArithmeticException.class)
public void shouldOverflowIfBigIntegerCannotFitInDouble() {
    NumberHelper.castTo(new BigInteger("1").shiftLeft(2000), N.double_);
}

[andreachild]

Same check is done for Float casting. Suggest to extract a common method for infinity/NaN check like:

private boolean isInfinityOrNaN(Number num) {
    return (!num.getClass().equals(BigDecimal.class) && !num.getClass().equals(BigInteger.class) &&
            (Double.isInfinite(num.doubleValue()) || Double.isNaN(num.doubleValue())));
}

[andreachild]

Suggest to add these test cases:

@Test
public void shouldCastDoubleInfinityToFloatInfinity() {
    assertEquals(Float.POSITIVE_INFINITY, NumberHelper.castTo(Double.POSITIVE_INFINITY, N.float_));
    assertEquals(Float.NEGATIVE_INFINITY, NumberHelper.castTo(Double.NEGATIVE_INFINITY, N.float_));
}

@Test
public void shouldCastFloatInfinityToDoubleInfinity() {
    assertEquals(Double.POSITIVE_INFINITY, NumberHelper.castTo(Float.POSITIVE_INFINITY, N.double_));
    assertEquals(Double.NEGATIVE_INFINITY, NumberHelper.castTo(Float.NEGATIVE_INFINITY, N.double_));
}

@Test
public void shouldCastFloatNaNToDoubleNaN() {
    assertEquals(Double.NaN, NumberHelper.castTo(Float.NaN, N.double_));
}

@Test
public void shouldCastDoubleNaNToFloatNaN() {
    assertEquals(Float.NaN, NumberHelper.castTo(Double.NaN, N.float_));
}

[andreachild]

Suggested change

	private static Long getLong(final Number num, final Class<? extends Number> clazz) {
	private static Long longValue(final Number num, final Class<? extends Number> targetClass) {

[andreachild]

Do we also need to check for underflow?