Skip to content

IsCloseTo matcher should operate on Number rather than Double#184

Open
mrpotes wants to merge 2 commits intohamcrest:masterfrom
mrpotes:is-close-to-number
Open

IsCloseTo matcher should operate on Number rather than Double#184
mrpotes wants to merge 2 commits intohamcrest:masterfrom
mrpotes:is-close-to-number

Conversation

@mrpotes
Copy link
Copy Markdown

@mrpotes mrpotes commented Aug 30, 2017

The IsCloseTo/closeTo matcher should be able to check all numerical types are within the bounds set, rather than just doubles. The implementation already worked if a primitive value was cast to double, but did not work in the case of boxed non-Double numbers, e.g. when trying to check Integer values in a list or map, etc.

@sf105 sf105 self-assigned this Aug 3, 2018
@sf105
Copy link
Copy Markdown
Member

sf105 commented Aug 3, 2018

Interesting. So the use case is that you have a collection of mixed numbers, some of which are floating point and some not?

@mrpotes
Copy link
Copy Markdown
Author

mrpotes commented Aug 8, 2018

Actually, IIRC I had a long, and wanted to do a close-to check, but it wouldn't work because number wasn't a double.

@sf105
Copy link
Copy Markdown
Member

sf105 commented Aug 8, 2018

In that case, it makes more sense to me to have an ordinal version of close to. It feels odd to do floating point comparisons for int/long.

@mrpotes
Copy link
Copy Markdown
Author

mrpotes commented Aug 8, 2018

I don't disagree with that as also desirable, but if you consider, for example, a number that is being parsed from JSON, you may not know what the eventual type is going to be (JSON not distinguishing between ordinal and floating point numbers), so it is still useful to have the floating point version deal with Number so that if a JSON field had either 123.456 or 123 in it, it would work regardless.

@sf105
Copy link
Copy Markdown
Member

sf105 commented Aug 10, 2018

Good point

@nhojpatrick
Copy link
Copy Markdown
Member

@mrpotes wanting to kick start open pr's, can you rebase from master, as hamcrest-core and hamcrest-library have been refactored a lot and also deprecated, so that everything is just in hamcrest.

@nhojpatrick
Copy link
Copy Markdown
Member

Going to try and kick start hamcrest, so if you want to get it merged, please rebase from the branch v2.3-candidates.
Still trying to understand how has permissions to perform a release.

@nhojpatrick nhojpatrick force-pushed the master branch 2 times, most recently from 9bc653b to e9f7fc8 Compare February 13, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Needs triage

Development

Successfully merging this pull request may close these issues.

3 participants