Comparing time is incorrect when picking 12:00

Question

I am creating a booking system and I don't want to allow users to book with starting time 11:00 and end time 09:00 (past)(I am using 24hour clock). I have two combo boxes filled with Strings that act as start and end time (09:00,10:00,11:00,12:00,13:00....)

I have this code:

 String start = (String) startTime.getSelectedItem();
        String end = (String) endTime.getSelectedItem();
        try {
            if(new SimpleDateFormat("hh:mm").parse(start).before(new SimpleDateFormat("hh:mm").parse(end))){
                System.out.println("test1");// future date - good

            }else{
                System.out.println("fail2");// old date - bad
            }
        } catch (ParseException ex) {
                System.out.println("error");
        }

This works perfectly except when I pick start/end time to be 12:00. Program outputs opposite of what it is supposed to output and I am unsure why.

If I pick start time 14:00 and end time 12:00 the program will output fail2(good output),

If I pick start time 09:00 and end time 12:00 the program will output fail2(should be test1),

if I pick start time 12:00 and end time 10:00 the program will output test1(should be fail2),

if I pick start time 12:00 and end time 15:00 the program will output test1(good output)

This type of problem only occurs when I pick 12:00..

I think your format needs to be `HH:mm` because `hh` refers to a 12-hour clock https://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html — , Apr 26 '18 at 12:26
Consider using `LocalTime time = LocalTime.parse(start);` for your times. — achAmháin, Apr 26 '18 at 12:29
I recommend you avoid the `SimpleDateFormat` class. It is not only long outdated, it is also notoriously troublesome. Today we have so much better in [`java.time`, the modern Java date and time API](https://docs.oracle.com/javase/tutorial/datetime/). Bonus points: As @notyou says, you won’t need an explicit formatter. And even if you did use a `DateTimeFormatter`, it would have notified you of your incorrect format pattern use through an exception. — Ole V.V., Apr 26 '18 at 12:52
Thank you guys for tips, I have to check this LocalTime out. — hDDen, Apr 26 '18 at 14:05
@OleV.V. upvoted but I meant on the old question you linked obviously.... TT_TT — tatsu, Apr 26 '18 at 15:30
Of course you meant that, @tatsu, sorry I misunderstood. Your call makes much better sense now. I have provided an answer to the linked question too. — Ole V.V., Apr 26 '18 at 16:01
Possible duplicate of [12:xx shown as 00:xx in SimpleDateFormat.format(“hh:mm:ss”)](https://stackoverflow.com/questions/49708445/12xx-shown-as-00xx-in-simpledateformat-formathhmmss). — Ole V.V., Apr 26 '18 at 16:02

score 7 · Answer 1 · edited Jun 20 '20 at 09:12

7

You should use HH:mm.

hh ranges 01-12, while HH ranges 1-23.

Update

Check the doc of SimpleDateFormat:

H: Hour in day (0-23)

h: Hour in am/pm (1-12)

edited Jun 20 '20 at 09:12

Community

1
1

answered Apr 26 '18 at 12:26

xingbin

27,410
9
53
103

This answer solves the issue too. Thank you @user6690200 – hDDen Apr 26 '18 at 14:05
@hDDen I have to remind you, in your answer, you are using `HH`... While in your question, it is `hh` – xingbin Apr 26 '18 at 14:06
I didnt know before that this makes a difference, now I know. Thank you again. – hDDen Apr 26 '18 at 14:09
1

@hDDen No problem :) – xingbin Apr 26 '18 at 14:11
This answer points out why your implementation fails, for practical purposes isn't the best solution as it disallows intervals that span midnight. It's a great answer to your question, but is not the best solution. – Jeff Learman Apr 26 '18 at 15:39

score 4 · Accepted Answer · edited Jun 20 '20 at 09:12

public static void checkTimes(String start, String end) {
    try {
        if (LocalTime.parse(start).isBefore(LocalTime.parse(end))) {
            System.out.println("test1");// future date - good
        } else {
            System.out.println("fail2");// old date - bad
        }
    } catch (DateTimeParseException dtpe) {
        System.out.println("error");
    }
}

Let’s try it:

    checkTimes("14:00", "12:00");
    checkTimes("09:00", "12:00");
    checkTimes("12:00", "10:00");
    checkTimes("12:00", "15:00");

This prints:

fail2
test1
fail2
test1

I believe this agrees with what you had intended. Note that LocalTime parses your strings without the need for an explicit formatter. Furthermore, if you trust that your combobox only contains valid time strings, you can leave out the try-catch construct since DateTimeParseException is an unchecked exception.

If startTime and endTime are JComboBox, I believe you can even fill LocalTime objects into them. Then you don’t need to parse when the user selects one from each. Your JComboBox will call LocalTime.toString(), which will return a string like 09:00, which in turn the combo box will display and let the user select.

    LocalTime[] times = { LocalTime.of(9, 0), LocalTime.of(10, 0), LocalTime.of(11, 0), 
                          LocalTime.of(12, 0), LocalTime.of(13, 0), LocalTime.of(14, 0) };
    JComboBox<LocalTime> combo = new JComboBox<>(times);

Unfolded:

I am using LocalTime from java.time, the modern Java date and time API. java.time is generally much nicer to work with than the old and outdated date and time classes like SimpleDateFormat, Date and more.

Link: Oracle tutorial: Date Time explaining how to use java.time.

This is a far better solution than one that can't support booking intervals that span midnight. Disallowing spanning midnight makes it much more complicated to support users from any time zone. The question is what do you really want: to compare *numbers* or to compare *times*? This is the solution that actually compares *times* rather than relatively arbitrary numbers that happen to represent portions of the time. (That arbitrariness is part of why your original solution didn't work.) — Jeff Learman, Apr 26 '18 at 15:36
Thank you so much for this great explanation of your answer. — hDDen, May 10 '18 at 13:46

hDDen · Answer 3 · 2018-04-26T14:05:00.340

0

String start = (String) startTime.getSelectedItem();
        String end = (String) endTime.getSelectedItem();
        SimpleDateFormat timer = new SimpleDateFormat("HH:mm");

        try {
            Date startD = timer.parse(start);
            Date endD = timer.parse(end);
            if(startD.before(endD)){
                System.out.println("test1");// future date - good
            }else{
                System.out.println("fail2");// old date - bad
            }
        } catch (ParseException ex) {
                System.out.println("error");
    }

Changing my code to this fixed this issue..

edited Apr 26 '18 at 14:05

answered Apr 26 '18 at 12:27

hDDen

145
1
2
13

3

No, @user6690200 's answer is the correct explanation. – andy Apr 26 '18 at 12:29
1

@andy Would it be possible that there are two answers providing the correct answer? And both of them using the long outdated and notoriously troublesome `SimpleDateFormat` class, BTW. Thank you anyway, hDDen, for providing your solution for us all. – Ole V.V. Apr 26 '18 at 13:39
2

@OleV.V. This answer is actually using `HH`, just like I posted. So they are basiclly the same one. – xingbin Apr 26 '18 at 14:08
@hDDen you first wrote that usage of two SimpleDateFormat instances solved the issue. I think you have deleted this sentence. That was what I was referring to. The solution is „HH:mm“ instead. So your answer is correct now. (Ignoring the fact of outdatedness of SimpleDateFormat) – andy Apr 26 '18 at 22:45

Comparing time is incorrect when picking 12:00

3 Answers3

Update

Linked

Related