BigDecimal equality using compareTo

[ac183]

Is your feature request related to a problem? Please describe.
BigDecimal can be used when exact decimal numbers are required rather than float or double floating point numbers which cannot represent all numbers exactly. When used as fields, if you want 1.0 to be considered equal to 1 or 1.00 then the equals method of the class containing the field should check equality of these fields using compareTo and ensure the same hashcode is used for all variants. There is currently not support in EqualsVerifier for checking this has been implemented correctly. So using EqualsVerifier alone is not enough for this use case.

Describe the solution you'd like
Ability to configure EqualsVerifier to give confidence that equals and hashCode are sufficient for two objects to be equal when any of their BigDecimal fields have the same numerical values even if the BigDecimal representation is unequal (due to number of decimal places).

Describe alternatives you've considered

• Implement this check in a separate library similar to/or wrapping EqualsVerifier e.g. BigDecimalEqualsVerifier, and use both/the wrapper to check classes with this use case. Currently do this but would be nice to have one library that can do all the equals verifying, hence this request.

Additional context

• I implemented a setting for this, pushed here, for your consideration - the manual page and integration test demonstrate the problem faced and solution. This solution adds a specific configuration method usingBigDecimalCompareTo().
• If this is a niche issue, then another idea is to add support for configuring custom field mutations? The implementation of BigDecimalFieldCheck is quite generic, the part specific to BigDecimal is this line to mutate the scale. Maybe this is a better approach as would support any other types that have inconsistency between compareTo and equals. Having said that, BigDecimal is the only type where I've had this problem.

[jqno]

Hi!

Good point, for some reason I've never considered BigDecimal really well. I completely agree with your assessment.

And I have to say, I'm impressed with the detail of your proposal. The implementation looks really nice, and you even added a manual page! I wish more people would do that :).

I do have a small doubt though: I feel like usingBigDecimalCompareTo should be the default. I think many people won't find usingBigDecimalCompareTo unless they start looking for something like it (as you obviously have), but after more than 10 years no-one came to me with this, so I fear not a great many people are looking :). So I would propose to make it the default behaviour, and then we could add a Warning.BIGDECIMAL_COMPARETO or something, which can be suppressed when the user wants to revert to the 'old' behaviour. This has the added benefit of keeping the API footprint smaller: I'm not fond of adding methods to EqualsVerifierApi, which I think is already a bit messy.

I think keeping it specific to BigDecimal is fine, at least for now.

What do you think?

[ac183]

Hi :)

Making this the default with the option to suppress sounds good to me.

I can make the changes and raise a PR for review? Do you need me to enable workflows to get some kind of green build?

I actually did it that way to start with but haven't worked on a truly public API or tried contributing before so was more cautious; thinking that adding it as configuration means it won't break anyone's existing tests and maybe few people care if this hasn't come up before.
However reading your response I can see there is another side which is: if anyone is affected (which will be only classes containing BigDecimal) then maybe it has not been considered and any broken tests raise the decision that needs to be made about how BigDecimals should be treated. This is an intentional improvement so behaviour change can be expected. Same as could happen with a bug fix.

Specific to BigDecimal is fine as I don't know any other cases... maybe in another 10 years!

[jqno]

I would very much appreciate a PR, if you're willing to do it!

When you open the PR, I have to approve the workflow run for new committers such as yourself, so that might take a little time. If you want to know sooner if it will be green, you can run mvn clean verify locally, which will perform the same verifications. The only extra thing the workflows do, is test on various JDK versions, but in this case I don't expect different outcomes. As long as you test on JDK 11 or 17, you should be good. It might complain about formatting, but if you run mvn spotless:apply, it will re-format automatically.

I always feel bad when someone comes with a PR they've clearly put a lot of work in, to ask for a lot of re-work. I hope you didn't mind :). I'm always open to discussing these things before you actually start putting the work in, though. I think most open source maintainers are. In any event, thanks for doing this, I look forward to your PR!

[jqno]

Thanks again for the nice PR! Version 3.8 is now syncing with Maven Central.