First, the method of getEndSLA
should return a Date
, or at least have some other method that does so, to improve object orientation. It should not be the responsibility of the comparison method to know how to convert strings to dates representing Slas.
So, assuming you can’t change the type of return getEndSLA
, recommend you add this from here in class Notificacao
:
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;
}
}
Thus, you can simplify your method compareTo
, and makes it 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 It’s not cool, it can result in something bizarre when the two dates are very different and some of the most significant bits are cut. In addition, we can compare the dates directly, without needing the getTime()
. Then I’ll fix it:
@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 method compareTo
can be perfectly used to sort the instances of Notificacao
date of end of SLA. The problem is when they don’t have them: return 0
will make them appear to be equivalent.
Let’s assume we have three objects Notificacao
. The object To with the date of Monday, B with the date of Tuesday and the C with the date null
. So according to the method compareTo
:
A.compareTo(A)
produces 0, hence A = A
B.compareTo(B)
produces 0, hence B = B
C.compareTo(C)
produces 0, hence C = C
A.compareTo(B)
produces -1, hence A < B
B.compareTo(A)
produces +1, so B > A
A.compareTo(C)
produces 0, hence A = C
C.compareTo(A)
produces 0, hence C = A
B.compareTo(C)
produces 0, hence B = C
C.compareTo(B)
produces 0, hence C = B
These last four lines are the problem, because if A = C
and C = B
, then we would conclude that A = B
. But no, yeah A < B
. That is, your method compareTo
is inconsistent, and therefore produces strange and/or incorrect results. This is where the contract of compareTo
is being raped.
The solution is to make the elements with date null
come or all before or all after those who have dates. I will put as all 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
C.compareTo(A)
produces -1, hence C < A
B.compareTo(C)
produces +1, so B > C
C.compareTo(B)
produces -1, hence C < B
And finally we have to C < A < B
, and there is no inconsistency in your compareTo
.