0

I have written custom comparator and test method to test it. But Whatever I do, I don't get 100% code coverage. It is not a big issue as I can live without 100% code coverage, but curious to know where I went wrong. I tried suggestion from some other question on stackoverflow of breaking comparator code on multiple lines like this but it didn't work. Also used & instead of && but no luck.

if (e1.getLastUpdateDateTime() == null 
&& e2.getLastUpdateDateTime() == null) 
return 0;

Code below for test method and comparator.

@Test
public void testCompare() {
    SomeObject m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(null);
    SomeObject m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(null);
    assertEquals(0, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(null);
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    assertEquals(1, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(null);
    assertEquals(-1, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    assertEquals(0, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:01"));
    assertEquals(-1, SomeUtil.compare(m1, m2));

    List<SomeObject> SomeObjectList = TestUtil.getSomeObjectListForComparator();
    SomeObject SomeObject = SomeObjectList.stream().sorted(SomeUtil::compare).findFirst().get();
    assertEquals("b", SomeObject.getSomeVariable());
}



public static int compare(SomeObject e1, SomeObject e2) {
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;
    return e2.getLastUpdateDateTime().compare(e1.getLastUpdateDateTime());
}

Jacoco Code Coverage

enter image description here

Thanks @Godin. I have now reimplemented using below code.

public static int compare(SomeObject e1, SomeObject e2) {
    return e1.getLastUpdateDateTime() == null
            ? (e2.getLastUpdateDateTime() == null ? 0 : 1)
            : (e2.getLastUpdateDateTime() == null ? -1 : e2.getLastUpdateDateTime().compare(e1.getLastUpdateDateTime()));
}
Sammy Pawar
  • 1,201
  • 3
  • 19
  • 38
  • That looks like 100% coverage to me, what is telling you it isn't? – Bentaye Jun 07 '22 at 09:58
  • @Bentaye Obviously, OP's code coverage tool. Hence the paste of the green and yellow lines stuff. – rzwitserloot Jun 07 '22 at 10:06
  • I think the tool does not analyze the `if`s as you expect. You could rewrite it in a big if-else-if construct. – Rob Audenaerde Jun 07 '22 at 10:07
  • @Bentaye - hover on yellow line says "1 of 6 branches missed". – Sammy Pawar Jun 07 '22 at 10:08
  • 2
    Nothing in this question sheds any light. Clearly the code you added to the question is not what you actually have (there are typos in them; they would not compile). Instead of immediately reaching for 'oh the tool must be broken' (they rarely are), you probably did something wrong but the part you messed up isn't anywhere in your question. – rzwitserloot Jun 07 '22 at 10:08
  • Thanks @Godin. I have now reimplemented using below code. public static int compare(SomeObject e1, SomeObject e2) { return e1.getLastUpdateDateTime() == null ? (e2.getLastUpdateDateTime() == null ? 0 : 1) : (e2.getLastUpdateDateTime() == null ? -1 : e2.getLastUpdateDateTime().compare(e1.getLastUpdateDateTime())); } – Sammy Pawar Jun 08 '22 at 21:15

1 Answers1

1

For

    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;

in the last if-statement

    if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;

to reach part after &&

e2.getLastUpdateDateTime() == null

preceding part before &&

e1.getLastUpdateDateTime() != null

should evaluate to true, but it never evaluates to false (i.e. e1.getLastUpdateDateTime() == null), because all possible cases of e1.getLastUpdateDateTime() == null are already in the two previous if-statements

    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;

so there is no way to write a test that will not reach part after && in the last if-statement. This is so-called dead code - code that can never be executed and this is exactly what the code coverage tool tries to tell you, because the code coverage tool shows what was executed and what was not. To change the result of the code coverage tool you can rewrite your code in a way that avoids dead code, for example

    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (/* e1.getLastUpdateDateTime() != null && */ e2.getLastUpdateDateTime() == null) return -1;
Godin
  • 9,801
  • 2
  • 39
  • 76