ImalkaPerera commented on PR #6651: URL: https://github.com/apache/fineract/pull/6651#issuecomment-6262518667
> Hi @ImalkaPerera, thanks for the PR. Using `compareTo` in `equals` and `stripTrailingZeros()` in `hashCode` keeps the contract consistent. > > Non-blocking: > > * Is the explicit zero branch in `hashCode` needed? Since Java 8 `BigDecimal.ZERO.stripTrailingZeros()` returns zero. > * The principal column is `nullable = false`. When can it be null here? If never, the null-safe helper and the null tests cover a state that cannot exist, and ten tests for a two-line change is a lot. Thanks for the review, @galovics ! Good points on both. You're right about the zero branch: since Java 8, `BigDecimal.ZERO.stripTrailingZeros()` already returns zero, so I removed the explicit zero check. Since `principal` is marked `nullable = false`, I also removed the null-safe helper and the null-related tests. I trimmed the test suite down to the four relevant cases: scale differences, zero values with different scales, different principal values, and different dates. I've pushed the updates. Please take another look when you get a chance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
