Add JsonNode.canConvertToExactIntegral() to indicate whether floating-point/BigDecimal values could be converted to integers losslessly

[oguzhanunlu]

Is your feature request related to a problem? Please describe.
DecimalNode might contain a valid integer or long, however DecimalNode.isIntegralNumber doesn't cover such cases and return false from parent JsonNode.isIntegralNumber .

Describe the solution you'd like
Implement DecimalNode.isIntegralNumber to return true if the value can fit into Long or Int

Usage example
DecimalNode.isIntegralNumber(-1) == true

Additional context
I want to use this check during json schema validation so that e.g. a DecimalNode containing an integer smaller than maximum long value can be considered as integer during type validation where schema has integer as type of the property

[cowtowncoder]

Hmmh. No, I don't think that would be good idea, changing semantics in a minor version.

However, there is canConvertToInt() and canConvertToLong() -- would these work?

[oguzhanunlu]

would these work?

Well, sort of, integral check can be accomplished by combining canConvertToLong and another predicate checking if the value has fractional part, since I don't want to interpret e.g. 12.88 as integral, even though it fits into a Long, so that'll do it.

Thanks for the quick reply and feel free to ping me if you'd like a PR for the next major release.

[cowtowncoder]

Ah, yes, checking for missing fractional part is sort of orthogonal, so those methods do not quite match what you are trying to do.

So it might be request for yet-another accessor (hasFractionalPart() or something). I am not sure if there are practical challenges wrt double/float backed nodes (it's trivial for BigDecimal as that is 10-based, not 2-based).

[oguzhanunlu]

so those methods do not quite match what you are trying to do

Yes that's the case and everyone who needs to check fraction will re-implement the same fractional check logic downstream.

An accessor like hasFractionalPart wouldn't change semantics and would come handy for sure, do you think it could be evaluated for a future minor release?

[cowtowncoder]

@oguzhanunlu Sure: I'll change the title and mark this as potentially good first issue for someone to consider implementing; and it could go in a new minor version (2.12 if contribution came quickly enough; 2.13 otherwise).

[bmsbharathi]

Hi
Can I work on this?

[cowtowncoder]

@bmsbharathi sorry for the slow response: looks like @siavashsoleymani has unfortunately already contributed a PR.

[cowtowncoder]

Ok, so, I think PR being worked on has functionality that will work on FloatNode / DoubleNode / DecimalNode; we can relatively efficiently check for necessary condition.

But now I wonder about naming of the method. Mostly since there's now the question of what to return in cases of:

1. Non-floating-point node? They don't really have fractions ever (so that kind of works)
2. FP value that is "not-a-number" (like positive/negative infinity, or NaN from division by zero) -- does not really have a fraction either -- but in most cases you would NOT want to try conversions from, say, +Infinity.

so since the intent is usually not to check for existence of fraction as much as whether node is losslessly convertible to integral value, this might lead to awkward checks since

I wonder should we not instead name method like:

• canConvertToIntegralValue() -- true for existing integer types, plus real FP values without fractional part, but false for non-numbers
• representsIntegralValue() / containsIntegralValue() -- same

or if we stick with check for fractions, switch it to instead use negation

• hasNoFractionalPart()

because then we could return true for integer nodes, as well as non-numeric things.
It may seem like minor difference (and some would think using negation in method name is a no-no), but I think it all comes down to what is most workable approach wrt actual expected usage.

WDYT?

[cowtowncoder]

Initial PR merged; will now ponder renaming -- leaning towards changing to something more like isOrConvertibleToIntegral(), which would return true if:

1. Natively integral numbers: JsonNode.isIntegralNumber() returns true
2. Coercible from something for which JsonNode.isNumber() returns true (in practice same as if JsonNode.isFloatingPointNumber() returns true)

[cowtowncoder]

As per title, my current thinking is that method to add will be named

canConvertToExactIntegral()

and will next check that version. But can still consider even better naming until 2.12.0-rc2.