CODEWITHSUNDEEP
javasortingcomparable
user623990
user623990

Reputation:

Sorting method not sorting correctly

I'm calling Arrays.sort(schedule, c); where c is an instance of a comparator like so:

import java.util.Comparator;

public class FirstOccComparator implements Comparator<AbstractEvent> {

    public int compare(AbstractEvent event1, AbstractEvent event2) {        
          int result = 0;       
          if (event1 == null || event2 == null) {
              //System.out.println("null");
          }
          else if (event1.hasMoreOccurrences() && event2.hasMoreOccurrences()) {
            result = event1.nextOccurrence().compareTo(event2.nextOccurrence());
          }
          return result;
    }

}

The output I'm getting isn't what it's supposed to be. I'm wondering if someone can point me in the right direction here. This is the first sorting algorithm I've ever made and its using concepts that are still new to me (comparators and implementation), so sorry about the multiple questions regarding my code :)

EDIT This is the difference between the outputs: http://pastebin.com/LWy1jqkt

There are two kinds of events, these are the hasMoreOccurrences() and nextOccurrence() methods:

DailyEvent

public boolean hasMoreOccurrences() {
        boolean result = false;
        Date check = nextOccurrence();
        timesCalled--;
        if (check instanceof Date && check != null) {
            result = true;
        }
        return result;
    }

public Date nextOccurrence() {
        if (timesCalled > recurrences) {
            return null;
        }
        else {
            Calendar cal = Calendar.getInstance();
            cal.setTime(startTime);
            cal.add(Calendar.DATE, timesCalled);
            timesCalled++;
            return cal.getTime();
        }
    }

WeeklyEvent

public boolean hasMoreOccurrences() {
        Date tmp = nextOccurrence();
        timesCalled--;
        boolean result = false;
        if (tmp instanceof Date && tmp != null) {
            result = true;
        }

        return result;
    }

public Date nextOccurrence() {
        Calendar cal = Calendar.getInstance();
        cal.setTime(startTime);
        cal.add(Calendar.DATE, timesCalled*7);

        if (cal.getTime().compareTo(this.endTime) > 0) {
            return null;
        }
        else {
            timesCalled++;
            return cal.getTime();
        }
    }

Upvotes: 0

Views: 260

Answers (4)

Mike Caron
Mike Caron

Reputation: 14561

Your algorithm for determining equality is deeply flawed. Read this for more details: http://blogs.msdn.com/b/oldnewthing/archive/2003/10/23/55408.aspx?wa=wsignin1.0

Upvotes: 0

Ted Hopp
Ted Hopp

Reputation: 234857

Ask yourself: what happens when event1 and event2 are not null but one (or both) has run out of occurrences?

Upvotes: 0

NG.
NG.

Reputation: 22914

There are a few things that seem incorrect with your comparator.

For instance, what happens if only one of them is null? How do you want those to sort? Right now you are considering two events equal if one of them is null.

Also, what happens if one event has more occurences while the other does not? Right now you only do comparisons on occurrences if both events have more occurrences. You need to handle the case where one has occurences while the other does not.

Also, if the occurence is a custom class, you need to evaluate that comparator as well.

Upvotes: 2

duffymo
duffymo

Reputation: 309008

When behavior doesn't match your assumptions, perhaps it's time to check your assumptions.

"...isn't what it's supposed to be..." suggests that you've got a notion of how your Comparator is supposed to work that isn't matching the output. Since the sorting algorithm built into Collections is proven, I think you need to look at your class and its Comparator for the error.

Write some unit tests and see where you've gone wrong.

Without seeing the results and the class you're sorting, it's impossible to advise you on what to do next.

Upvotes: 1

Related Questions