0

I have to sort an arraylist by the date and time entered by the user but for some reason the output comes out not in order

Below this code it the code im using to order

public int compareTo(Vehicle v){
    int returnValue = 0;
    if (this.parkDate.year> v.parkDate.getYear() && 
            this.parkDate.month> v.parkDate.getMonth() &&
            this.parkDate.day> v.parkDate.getDay() &&
            this.parkDate.hours> v.parkDate.getHours() &&
            this.parkDate.minuets> v.parkDate.getMinuets()){
        returnValue =  1; }
    else
        returnValue = - 1;
    return returnValue;
}

Here is the output message

Improve this question
2
  • 1
    You should really store Calendar or Date objects, not integers for each datetime part yourself... Commented Nov 14, 2016 at 3:38
  • well, if everything is the same but one minute is greater, that statement would return -1 instead of 1. You need to consider equal cases, and there's hierarchy here (if a month is less, it doesn't matter if it's a year ahead). You should use one of the available time apis in java (Date, joda time, etc) Commented Nov 14, 2016 at 3:49

5 Answers 5

Reset to default
2

Your comparison logic isn't correct. You might perform your comparisons with Integer.compare(int, int) and return the result in the case of non-zero. Something like,

public int compareTo(Vehicle v) {       
    int returnValue = Integer.compare(this.parkDate.getYear(), 
            v.parkDate.getYear());
    if (returnValue != 0) {
        return returnValue;
    }
    returnValue = Integer.compare(this.parkDate.getMonth(), 
            v.parkDate.getMonth());
    if (returnValue != 0) {
        return returnValue;
    }
    returnValue = Integer.compare(this.parkDate.getDay(), 
            v.parkDate.getDay());
    if (returnValue != 0) {
        return returnValue;
    }
    returnValue = Integer.compare(this.parkDate.getHours(), 
            v.parkDate.getHours());
    if (returnValue != 0) {
        return returnValue;
    }
    return Integer.compare(this.parkDate.getMinuets(), 
            v.parkDate.getMinuets());
}

Or, you could shorten the above by using arrays and something like

int[] a = { this.parkDate.getYear(), this.parkDate.getMonth(), 
        this.parkDate.getDay(), this.parkDate.getHours(), 
        this.parkDate.getMinuets() };
int[] b = { v.parkDate.getYear(), v.parkDate.getMonth(), 
        v.parkDate.getDay(), v.parkDate.getHours(), 
        v.parkDate.getMinuets() };
for (int i = 0; i < a.length; i++) {
    int rv = Integer.compare(a[i], b[i]);
    if (rv != 0) {
        return rv;
    }
}
return 0;

Finally, I believe you want minutes not minuets.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

The primary issue is your comparison logic. You are testing that your vehicle's park date's year is greater than the other vehicle's park date's year and your vehicle's park date's month is greater than the other vehicle's park date's month and ditto for the day, hour and minute. This is incorrect.

Consider 2016-01-01 00:00 and 2015-12-31 23:59. The former clearly comes after the latter, but its month, day, hour and minute are all less than the other. Your logic would therefore fail to produce the correct result in this instance.

A more suitable approach would be to:

  • compare the years: if they are different you can return a result, ie. less than or greater than
  • if they are the same, compare the months; if they are different you can return a result
  • if they are the same, compare the days; if they are different you can return a result
  • if they are the same, compare the hours; if they are different you can return a result
  • if they are the same, compare the minutes; if they are different you can return a result
  • if they are the same, indicate the dates are equal

(Also, make sure you remembered to implement Comparable on your Vehicle class.)

Improve this answer

2 Comments

Yes I have done Implement Comparable and I get what your saying and have tried to change it but it just comes up with errors Is there anything you can suggest
@AlexBurrows I've updated my answer with a pseudo-coded solution.
0

The reason is because you have an error in your boolean logic.

if (this.parkDate.year> v.parkDate.getYear() && 
        this.parkDate.month> v.parkDate.getMonth() &&
        this.parkDate.day> v.parkDate.getDay() &&
        this.parkDate.hours> v.parkDate.getHours() &&
        this.parkDate.minuets> v.parkDate.getMinuets()){
    returnValue =  1; }
else
    returnValue = - 1;

Else is like taking the negation of your if statement. In this case that equates to the following:

this.parkDate.year <= v.parkDate.getYear()

OR

this.parkDate.month <= v.parkDate.getMonth()

OR

this.parkDate.day> v.parkDate.getDay()
...

In other words 

Negation(A && B) <=> (Negation(A) || Negation(B))

In the context of your situation, your else logic could be true even if the year is greater than this.parkDate

I think this should be enough to help you get it right. :)

Improve this answer

Comments

0

i think you can use "compareTo" the result is 0, 1, -1

public int compareTo(Vehicle a, Vehicle b){
    return a.parkDate.compareTo(b.parkDate);
}
Improve this answer

Comments

0

Instead of storing the date and time as primitives, you can create a LocalDateTime and use it in Comparator

add field parkDate as LocalDateTime in Vehicle class 

LocalDateTime parkDate = LocalDateTime.of(year, month, dayOfMonth, hour, minute, second);

use the parkDate in compareTo method

@Override
public int compareTo(Vehicle o) {       
    return this.parkDate.compareTo(o.parkDate);
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.