patternjavaMinor
Checking if a date comes after another in Java
Viewed 0 times
aftercheckingdatejavaanothercomes
Problem
I need to test if a date is posterior to another in java, I just need to be sure that my approach is a good one
public class Date {
private int day;
private int month;
private int year;
public Date(int day, int month, int year) {
this.day = day;
this.month = month;
this.year = year;
}
public int getDay() {
return day;
}
public int getMonth() {
return month;
}
public int getYear() {
return year;
}
public boolean posterieurTo(Date dateToCompare){
if (year > dateToCompare.getYear())
return true;
else if (month > dateToCompare.getMonth())
return true;
else if (day > dateToCompare.getDay())
return true;
else
return false;
}
}Solution
The obvious
As mentioned in @mtj's comment, we have three different JDK APIs for doing this, so you really shouldn't have to resort to reinventing-the-wheel.
Naming
Ok, maybe this is some simple assignment that you need to do... how about the naming? You should try to avoid using the same class name from the JDK, as it will get messy understanding which class is actually being used. For example, the wrong usage of your class might lead to confusing
Also, since you are already using English mostly, you should translate that French-sounding method name to the English equivalent too, e.g.
Validation
You should be checking whether
Next, assuming you have renamed this class, you should consider handling
Boolean expressions
Finally, you can chain the boolean expressions together using
Admittedly, this is getting hard to comprehend.
(thanks to @JS1's answer for the update)
As mentioned in @mtj's comment, we have three different JDK APIs for doing this, so you really shouldn't have to resort to reinventing-the-wheel.
java.util.Date.after(Date)
java.util.Calendar.after(Object)
java.time.LocalDate.isAfter(ChronoLocalDate)
Naming
Ok, maybe this is some simple assignment that you need to do... how about the naming? You should try to avoid using the same class name from the JDK, as it will get messy understanding which class is actually being used. For example, the wrong usage of your class might lead to confusing
toString() representation, or syntax errors when trying to do a Date.getTime().Also, since you are already using English mostly, you should translate that French-sounding method name to the English equivalent too, e.g.
isAfter(). Whether you should have your entire codebase in non-English is another debating topic altogether. :)Validation
You should be checking whether
day and month are valid or not... you wouldn't want your bespoke class to start representing the 99th day of the 555th month of the 18288th year... assuming we're still talking about the Greogorian calendar.null checkNext, assuming you have renamed this class, you should consider handling
null properly: do you want to throw a NullPointerException as it is currently, or return a default value?Boolean expressions
Finally, you can chain the boolean expressions together using
&& and || so that the method returns true the moment any of the expression is true too:return dateToCompare != null && // assuming returning false for null values
year > dateToCompare.getYear() || (year == dateToCompare.getYear() &&
(month > dateToCompare.getMonth() || (month == dateToCompare.getMonth() &&
day > dateToCompare.getDay())));Admittedly, this is getting hard to comprehend.
(thanks to @JS1's answer for the update)
Code Snippets
return dateToCompare != null && // assuming returning false for null values
year > dateToCompare.getYear() || (year == dateToCompare.getYear() &&
(month > dateToCompare.getMonth() || (month == dateToCompare.getMonth() &&
day > dateToCompare.getDay())));Context
StackExchange Code Review Q#155268, answer score: 5
Revisions (0)
No revisions yet.