5

I have this:

// returns true if both are equal (independent of scale) and also checks against null
public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
        // 1. check: both will be null or both will be non-null.
        if (val1 != null ^ val2 != null) return false;
        // 2. check: if not null, then compare if both are equal
        return !(val2 != null && val1.compareTo(val2) != 0);
    }

I want to merge the boolean expressions into one. So I use this:

public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return !(val1 != null ^ val2 != null) && !(val2 != null && val1.compareTo(val2) != 0);
}

However, I am worry if this is correct. Is this correct? Could this be simplified/shorten?

With the help of the answer the shorten solution is:

// returns true, if both are null or both are equal 
// (independent of their numbers scales)
public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return val1 == null ? val2 == null : val2 != null && val1.compareTo(val2) == 0;
}
nimo23
  • 5,170
  • 10
  • 46
  • 75
  • So you want to check if two BigDecimal are equals? – Dani Mesejo Sep 20 '19 at 17:23
  • 2
    _"I want to merge the boolean expressions into one."_ -- Don't. I fully understand the desire to have more "compact" code, but the compiler is going to convert them both into the same bytecode, so you should stick to making your code as easy to read (and thus to debug and maintain) as possible. – Jordan Sep 20 '19 at 17:26
  • What do you mean by "correct"? Does it accomplish the logic or functionality you want? – TylerH Oct 03 '19 at 21:21
  • @TylerH yes as it was answered.. – nimo23 Oct 03 '19 at 21:23
  • @nimo23 Sorry, that doesn't make sense. By "it" I mean the code in your question. If the code in your question works as you expect, then it is "correct". Otherwise you need to clarify what you mean (or the question risks being closed as unclear/opinion-based). – TylerH Oct 03 '19 at 21:24
  • yes, the code in my question was already right but too complicated..the shorten solution simplifies it. – nimo23 Oct 03 '19 at 21:26

3 Answers3

7

Using ^ as a logical XOR operator is highly unusual. It works, but I would avoid it for readability's sake. != is a good replacement.

return !((val1 != null) != (val2 != null)) && !(val2 != null && val1.compareTo(val2) != 0);

Now you can replace the double negation with ==. Nice.

return ((val1 != null) == (val2 != null)) && !(val2 != null && val1.compareTo(val2) != 0);

You can also distribute the remaining ! via De Morgan's laws:

return ((val1 != null) == (val2 != null)) && (val2 == null || val1.compareTo(val2) == 0);

It's better, but to be honest I still find it unwieldy. I'd go with something more straightforward even if it's not a single line statement:

if (val1 == null) {
    return val2 == null;
}
else {
    return val2 != null && val1.compareTo(val2) == 0;
}

You could use the ternary operator in place of if/else. Up to personal preference which you find more readable:

return val1 == null
    ? val2 == null
    : val2 != null && val1.compareTo(val2) == 0;

You mention that you need to use compareTo(). For others who might read this answer, if you don't have to use compareTo() I'd use equals() instead.

if (val1 == null) {
    return val2 == null;
}
else {
    return val1.equals(val2);
}

Then, as it so happens, you wouldn't even need to write this method at all. The built-in Objects.equals() method does exactly this: it returns true if two objects are equal, or if they're both null.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
  • 1
    `a.equals(b)` behaves the same as `Objects.equals(a, b)` which is NOT what I want. I want to check if both are equal (**independent of scale**) and also checks against nulls..(so if both are null, both should be treated as equal. – nimo23 Sep 20 '19 at 17:29
  • @Rogue I dont understand: I want to use `boolean isEqual(BigDecimal val1, BigDecimal val2) {return val1 == null ? val2 == null : val2 != null && val1.compareTo(val2) == 0;}`. How would you shorten this? – nimo23 Sep 20 '19 at 17:48
  • @nimo23 I think the last example/code of this answer was meant, not your comment – user85421 Sep 20 '19 at 17:49
2

You could use Comparator.nullsFirst (or Comparator.nullsLast):

public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return Comparator.nullsFirst(BigDecimal::compareTo).compare(val1, val2) == 0;
}

Example:

    System.out.println(isEqual(null, null));
    System.out.println(isEqual(null, new BigDecimal(1.0)));
    System.out.println(isEqual(new BigDecimal(1.0), null));
    System.out.println(isEqual(new BigDecimal(1.0), new BigDecimal(1.0)));

Output

true
false
false
true
Dani Mesejo
  • 61,499
  • 6
  • 49
  • 76
  • 1
    hmm..I like your solution..however I guess, the approach of the accepted answer is better suited for overiding `equals()/hashCode()` methods..no need to create objects Comparator..etc..only boolean operators..should be faster.. – nimo23 Sep 20 '19 at 18:40
2

If scale wasn't an issue, I would utilize a ternary here, and take advantage of Object#equals returning false for null:

//Basically Objects#equals at this point
public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return val1 == null ? val2 == null : val1.equals(val2);
}

Thus in every case:

[1: null, 2: null]: true
[1: null, 2:  num]: false
[1: num,  2: null]: false
[1: num,  2:  num]: Object#equals

However as you've noted, you want to compare irrespective of scaling as well. So we'll have to include another null check as #compareTo throws an NPE if you pass null:

public static boolean isEqual(BigDecimal val1, BigDecimal val2) {
    return val1 == null
            ? val2 == null
            : val2 != null && val1.compareTo(val2) == 0;
}

Thus in every case (again):

[1: null, 2: null]: true
[1: null, 2:  num]: false
[1: num,  2: null]: false
[1: num,  2:  num]: BigDecimal#compareTo

I would find this more readable, but it's still a little unweildly. At this point, a well-written and succinct comment (or, breaking up the line per the other answer) would probably do you better than code rewriting.

Rogue
  • 11,105
  • 5
  • 45
  • 71
  • Thanks, pitty that JDK does not provide the null checks within `compareTo()` out of box.. – nimo23 Sep 20 '19 at 18:00
  • 1
    It really shouldn't, as you're returning a scalar which represents the difference between two values. There's no good number to show the difference between `14` and `null` that wouldn't either conflict with other similar values or introduce a magic value. – Rogue Sep 20 '19 at 18:03
  • But overwriting `equals()` / `hashcode()` with `BigDecimal` is actually a little risky because someone needs to know that 1.0 != 1.000 by default when using `Objects.equals(1.0, 1.000)` and then I must treat NPE ..so the code above is not easy to have when overriding equals/hashCode for BigDecimal properties.. – nimo23 Sep 20 '19 at 18:08
  • first code snippet is exactly what `Objects#equals` does... no need to re-implement it – user85421 Sep 20 '19 at 18:09
  • @CarlosHeuberger good point, I'll leave it up for posterity. An alternative solution may be setting the scales to match, but I don't like the side effects of that solution. – Rogue Sep 20 '19 at 18:18
  • I don't like the idea of changing scale, to easy to mess up – user85421 Sep 20 '19 at 19:24