Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add assertion for floating point number format #789

Open
kiwi-oss opened this issue Jul 5, 2024 · 3 comments
Open

Add assertion for floating point number format #789

kiwi-oss opened this issue Jul 5, 2024 · 3 comments

Comments

@kiwi-oss
Copy link

kiwi-oss commented Jul 5, 2024

Read the documentation, maybe we already have the feature
There's a section about numerical comparisons, but I don't need to compare numbers, I need to determine the type.

There's a section about type placeholders, but it only mentions asserting the number type without distinguishing numbers with or without fractional part.

Is your feature request related to a problem? Please describe.
I need to assert that a number has a fractional part, e.g. 653.401 or even 12.0, but not 12.

Describe the solution you'd like
There's already JsonAssert#isIntegralNumber, but there's no negated counterpart. So one idea would be to add JsonAssert#isNotIntegralNumber.

This has the downside that the name leaves room for the interpretation that non-numbers like strings would also pass the assertion. To mitigate that, one could pick a name like isNumberWithFractionalPart. But then isIntegralNumber would have to be renamed to isNumberWithoutFractionalPart accordingly.

I'd find it even more intuitive to write JsonAssertions.assertThatJson("1.0").isNumber().hasFractionalPart() and .doesNotHaveFractionalPart() accordingly, but this would have to be added to AssertJ's BigDecimalAssert.

Describe alternatives you've considered
I first used .isNumber().scale().isNotZero() as workaround, but the failure message is confusing because it refers to the scale, not the checked number. Adding withFailMessage isn't that useful as it doesn't allow printing the checked number.

Now I'm using .isNumber().has(new Condition<>(bd -> bd.scale() != 0, "a fractional part")) and that's fine for now, but I think users shouldn't be forced to write an own solution when the negated assertion is already available.

@lukas-krecan
Copy link
Owner

Hi, thanks for feedback. Just a clarification question, to what categories would you put 10e3 or 1.2e12? Can you try to file a similar ticket to AssertJ, adding it there would IMO help the most.

@kiwi-oss
Copy link
Author

kiwi-oss commented Jul 8, 2024

Thanks for the question! It made me a realise that my suggestions for the method names aren't precise enough. Both 10e3 and 1.2e12 should pass the assertion I'm looking for, i.e. isNumberWithFractionalPart or hasFractionalPart should return true.

I basically want to know whether the number is provided in a floating point syntax. I saw two possible points of confusion that I wanted to avoid in the name:

  • 12.0 being mathematically equivalent to 12 could be seen as an "integral number"
  • Names like isFloat or isInteger suggest a relation to the corresponding Java types, but JSON numbers don't have the size and precision limitations of float and int

Therefore, I focussed on the fractional part in the names but forgot about the exponential "e" notation.

So for clarity, my updated propositions are JsonAssert#isNumberInFloatingPointFormat and JsonAssert#isNumberInIntegerFormat, but I will file a corresponding AssertJ ticket.

@lukas-krecan
Copy link
Owner

Cool, if the AssertJ team decides they don't want to add it, I can do it on JsonUnit side

@kiwi-oss kiwi-oss changed the title Add assertion for numbers with a fractional part Add assertion for floating point number format Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants