First, the getEndSLA
method should return a Date
, or at least have some other method that does, to improve object orientation. It should not be the responsibility of the comparison method to have to know how to convert strings to dates representing SLAs.
So, assuming you can not change the return type of
getEndSLA
, I recommend you add this from here in the
Notificacao
class:
public Date getEndSLADate() {
String s = getEndSLA();
if (s == null || s.isEmpty()) return null;
SimpleDateFormat format = new SimpleDateFormat("dd/MM/yyyy HH:mm",
Locale.getDefault());
try {
return format.parse(s);
} catch (ParseException e) {
e.printStackTrace();
return null;
}
}
So you can simplify your compareTo
method, and it's easier to understand, analyze, and improve it:
@Override
public int compareTo(Notificacao another) {
Date a = getEndSLADate();
Date b = another.getEndSLADate();
if (a != null && b != null) return (int) (a.getTime() - b.getTime());
return 0;
}
Much simpler, right?
That cast is not cool, it can result in something bizarre when the two dates are very different and some of the more significant bits are cut off. Also, we can compare dates directly, without needing getTime()
. So I'll sort this out:
@Override
public int compareTo(Notificacao another) {
Date a = getEndSLADate();
Date b = another.getEndSLADate();
if (a != null && b != null) return a.compareTo(b);
return 0;
}
Clearly, the compareTo
method can be perfectly used to sort instances of Notificacao
that have the SLA end date . The problem is when they do not have them: return 0
will make them appear to be equivalent.
Let's suppose we have three objects Notificacao
. The A with the date of Monday, the B with the date of Tuesday and the C with the date null
. Then, according to the compareTo
method:
A.compareTo(A)
produces 0, so A = A B.compareTo(B)
produces 0, so B = B C.compareTo(C)
produces 0, so C = C A.compareTo(B)
produces -1, logo A < B
B.compareTo(A)
produces +1, so B > A A.compareTo(C)
produces 0, so A = C C.compareTo(A)
produces 0, so C = A B.compareTo(C)
produces 0, so B = C C.compareTo(B)
produces 0, so C = B
These last four lines are the problem, because if A = C
and C = B
, then we conclude that A = B
. But no, because A < B
. That is, its compareTo
method is inconsistent, so it produces strange and / or incorrect results. This is where the compareTo
contract is being violated.
The solution is to make elements with date null
come or all before or all after dates. I'll put like everyone before:
@Override
public int compareTo(Notificacao another) {
Date a = getEndSLADate();
Date b = another.getEndSLADate();
if (a == null && b == null) return 0;
if (a == null) return -1;
if (b == null) return 1;
return a.compareTo(b);
}
And now we have this:
A.compareTo(C)
produces +1, so A > C with C.compareTo(A)
produces -1, so C < The% w / w of% produces +1, so B > C with B.compareTo(C)
produces -1, so C < B
And finally we have C.compareTo(B)
, and there is no inconsistency in your C < A < B
.