blue-sky
blue-sky

Reputation: 53786

Sort date/time objects (latest first)

I'm trying to order date objects by date/time (latest first) using code below. The ordering is not correct. I think the compareTo method needs to be implemented differently to achieve the required sorting ?

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;


public class DateSorter {

    public static void main(String args[]){

        List<DateObject> list = new ArrayList<DateObject>();

        DateObject d1 = new DateObject("2012-12-05" , "11:21:19");
        list.add(d1);

        d1 = new DateObject("2012-12-05" , "11:20:19");
        list.add(d1);

        d1 = new DateObject("2012-12-05" , "11:20:19");
        list.add(d1);

        d1 = new DateObject("2012-12-04" , "10:20:19");
        list.add(d1);

        d1 = new DateObject("2010-12-07" , "13:20:19");
        list.add(d1);

        d1 = new DateObject("2012-12-05" , "11:20:19");
        list.add(d1);

        Collections.sort(list);

        for(DateObject d : list){
            System.out.println(d);
        }
    }

}

import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;

public class DateObject implements Comparable<Object> {

    private String date;
    private String time;

    public DateObject(String date, String time) {
        this.date = date;
        this.time = time;
    }

    public int compareTo(Object o) {

        DateFormat formatter;
        Date date1 = null;
        Date date2 = null;
        formatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
        try {
            date1 = (Date) formatter.parse(this.date + " " + this.time);
            date2 = (Date) formatter.parse(this.date + " " + this.time);
        } catch (ParseException e) {
            e.printStackTrace();
        }
        catch(NullPointerException npe){
            System.out.println("Exception thrown "+npe.getMessage()+" date1 is "+date1+" date2 is "+date2);
        }

         return date1.compareTo(date2);

    }

    @Override
    public String toString(){       
        return this.date+" "+this.time;
    }

}

This is the output displayed :

2012-12-05 11:21:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-04 10:20:19
2010-12-07 13:20:19
2012-12-05 11:20:19

This output should be :

2010-12-07 13:20:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-05 11:21:19
2012-12-04 10:20:19

Update : when I modify the compareTo method to :

public int compareTo(Object o) {


    DateFormat formatter;
    Date date1 = null;
    Date date2 = null;  
    DateObject other = (DateObject) o;

    formatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
    try {
        date1 = (Date) formatter.parse(this.date + " " + this.time);
        date2 = (Date) formatter.parse(other.date + " " + other.time);
    } catch (ParseException e) {
        e.printStackTrace();
    }
    catch(NullPointerException npe){
        System.out.println("Exception thrown "+npe.getMessage()+" date1 is "+date1+" date2 is "+date2);
    }

     return date1.compareTo(date2);

}

The output is :

2010-12-07 13:20:19
2012-12-04 10:20:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-05 11:21:19

Which is still incorrect as 2012-12-05 should appear before 2012-12-04

Upvotes: 3

Views: 10356

Answers (5)

assylias
assylias

Reputation: 328568

You are parsing the same date twice:

date1 = (Date) formatter.parse(this.date + " " + this.time);
date2 = (Date) formatter.parse(this.date + " " + this.time);

should probably be:

date1 = (Date) formatter.parse(this.date + " " + this.time);
DateObject other = (DateObject) o;
date2 = (Date) formatter.parse(other.date + " " + other.time);

and you should probably test that o is a DateObject and not null before trying to cast it and use it.

EDIT

Just tried your updated code using return date2.compareTo(date1); and I get:

2012-12-05 11:21:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-05 11:20:19
2012-12-04 10:20:19
2010-12-07 13:20:19

which is in descending order (note that the last date is in 2010).

Upvotes: 5

Gustav Grusell
Gustav Grusell

Reputation: 1176

DateObject should implement Comparable. In your compareTo method, date2 needs to be derived from the object passed into the method.

Upvotes: 0

Andrii Polunin
Andrii Polunin

Reputation: 1306

You should do something like this:

DateObject other = (DateObject) o;

date1 = (Date) formatter.parse(this.date + " " + this.time);
date2 = (Date) formatter.parse(other.date + " " + other.time);

date2.compareTo(date1);

Notice the last line. You say you want the latest to be the first. In this case you should compare date2 to date1 not vice versa, because date1.compareTo(date2) will sort in the ascending order.

Upvotes: 0

Slauster
Slauster

Reputation: 99

I think the error is :

 date2 = (Date) formatter.parse(this.date + " " + this.time);

should be

 date2 = (Date) formatter.parse(((DateObject)o).date + " " + ((DateObject)o).time);

and you can also do this to get rid of the cast :

public class DateObject implements Comparable<DateObject> {
...
public int compareTo(DateObject o) {
    date2 = (Date) formatter.parse(o.date + " " + (o.time);

Upvotes: 0

Brian Agnew
Brian Agnew

Reputation: 272207

You're comparing the date to itself, not the object passed in

 date1 = (Date) formatter.parse(this.date + " " + this.time);
 date2 = (Date) formatter.parse(this.date + " " + this.time);

Where's o used ?

What does DateObject give you, btw ? It looks like a Date object. Note that java.util.Date holds date and time info, confusingly. If you use a standard date object then you don't have to write a comparator. Better still, you could investigate Joda-Time, which has a better, more intuitive and thread-safe API.

Upvotes: 0

Related Questions