How to write a Comparator

Question

public class Date {
    private int month; // must be 1-12
    private int day; // must be 1-31

    public int getMonth() {return month;}

    public int getDay() {return day;}

    public Date(int m, int d) {
        if (m >= 1 && m <= 12)
            month = m;
        else
            month = 1;

        if (d >= 1 && d <= 31)
            day = d;
        else
            day = 1;
    } // end constructor
} // end class Date

Comparator Class

import java.util.*;
public class DateComparator implements Comparator <Date>{
    public int compare(Date date1, Date date2){
            if (date1.getMonth() > date2.getMonth()){
                return 1;
            }
            else if(date1.getMonth() < date2.getMonth()){
                return -1;
            }
            else{ //if(date1.getMonth() == date2.getMonth()){
                if (date1.getDay() > date2.getDay()){
                    return 1;
                }
                if (date2.getDay() < date2.getDay()){
                    return -1;
                }
                else{// (date2.getDay() == date2.getMonth()){
                    return 0;
                }
            }
        }
    }

I'm trying to write a comparator for this date class, and I was wondering if this was the correct way to do it. Any advice would be much appreciated!

Solution

First of all, this question seems to be more appropriate for Code Review. But there are some concepts to explain that go beyond code review, so I decided to post an answer.

First draft

Your comparator can be considered as a first draft. It is working well and compares two Date objects as specified. Well done.

Code improvement

The many if-else-statements make the comparator somewhat clumsy and unreadable. Keep in mind that the compare method is not bound to returning -1, 0, or 1. It can return any negative number if the first argument is less than the second one, and any positive number if the first argument is greater than the second one. Only the 0 return is bound to equality.

As month and day are both represented as integers, you can simply use them in a little arithmetic. The month difference is more important - it weighs more - so it must be heavier:

public int compare(Date date1, Date date2) {
    int monthDiff = date1.getMonth() - date2.getMonth();
    int dayDiff = date1.getDay() - date2.getDay();
    return monthDiff * 100 + dayDiff;
}

Subtractions already produce a negative number, a zero, or a positive number. So use them. The factor 100 makes the month difference more important than the day difference.

If the month difference is not 0, then adding the day difference will not have any effect (because of the factor 100). Only when the month difference is 0, then the day difference will be important.

Code structure

Comparing two dates this way looks very natural. In fact, this is a natural ordering on dates. If a type has such a natural ordering, you should (must) let it implement Comparable:

public class Date implements Comparable<Date> {
    ...
    @Override
    public int compareTo(Date other) {
        int monthDiff = this.getMonth() - other.getMonth();
        int dayDiff = this.getDay() - other.getDay();
        return monthDiff * 100 + dayDiff;
    }
}

Other comparators

If you feel that you must have some other comparators, you can always add them. A good place is a nested static class inside your Date class (as they simply belong to it).

Let's make a comparator that only take the month into account:

public class Date implements Comparable<Date> {
    ...
    public static final class MonthComparator implements Comparator<Date> {
        @Override
        public int compare(Date date1, Date date2) {
            return date1.getMonth() - date2.getMonth();
        }
    }
}

Comments

[tmucha]

Not exactly, the contract says: compare methods returns a negative integer, zero, or a positive integer as the first argument is less than, equal to, or greater than the second.