3

I have an unfinished Android app with a main activity with a TimePicker (@+id/tyme), a DatePicker (@+id/date), and a TextView (@+id/dropTime) for displaying a "DateTime"-like datum that for some reason I need two views to specify.

Because the Javadocs deprecate all attempts to extract DateTime-field-type values from a Date instance, I thought to use the Calendar class to represent the user-selected combination of year,month,day,hour,minute,second. I find that I often (but not always) can't decrement the hour or minute fields without simultaneously incrementing the day-of-month field. Here's my event handler for the DatePicker:

        date = (DatePicker) findViewById(R.id.date);
        // h/t O'one https://stackoverflow.com/a/34952495/948073
        date.init(
                calendar.get(Calendar.YEAR),
                calendar.get(Calendar.MONTH),
                calendar.get(Calendar.DAY_OF_MONTH),
                new DatePicker.OnDateChangedListener() {
                    @Override
                    public void onDateChanged(DatePicker datePicker, int y, int m, int d) {
                        System.out.println("onDateChanged(tp,"+y+","+m+","+d+")");
                        calendar.set(Calendar.YEAR, y);
                        calendar.set(Calendar.MONTH, m);
                        calendar.set(Calendar.DAY_OF_MONTH, d);
                        updateDropTime();
                    }
                }
        );

And the TimePicker. Note the explicit backup-and-restore of the y/m/d fields in a vain attempt to stamp out suspected cross-contamination of y:m:d during h:m:s changes:

        private TimePicker time;
        dropTime = (TextView) findViewById(R.id.dropTime);
        updateDropTime();
        time = (TimePicker) findViewById(R.id.tyme);


        time.setOnTimeChangedListener(new TimePicker.OnTimeChangedListener() {
            public void onTimeChanged(TimePicker tp, int h, int m) {
                System.out.println("onTimeChanged(tp,"+h+","+m+")");
                // Save state of y/m/d portion of calendar
                int y = calendar.get(Calendar.YEAR);
                int mo = calendar.get(Calendar.MONTH);
                int d = calendar.get(Calendar.DAY_OF_MONTH);
                // Set h and m fields of calendar based on user input
                calendar.set(Calendar.HOUR, h);
                calendar.set(Calendar.MINUTE, m);
                // Restore state of y/m/d fields
                calendar.set(Calendar.YEAR, y);
                calendar.set(Calendar.MONTH, mo);
                calendar.set(Calendar.DAY_OF_MONTH, d);
                // In theory y/m/d portion of datetime should be unchanged
                updateDropTime();
            }
        });

Both these event handlers call updateDropTime(), which looks like this:

private void updateDropTime() {
    String disp = DateFormat.getDateTimeInstance().format(calendar.getTimeInMillis());
    dropTime.setText(disp);
}

I suspect that something in the method chain leading to a value for disp is where the seemingly chaotic behavior takes place, seeing as I would think I've systematically stamped out any changes to date fields that could conceivably be side effects of changes to time fields.

Lori
  • 1,392
  • 1
  • 21
  • 29
  • 1
    Weird. Is the behavior deterministic? I mean, can you find an example calendar value and example `h` and `m` values that consistently produce the increment of the day-of-month field? – Ole V.V. Jul 23 '17 at 03:08
  • `Calendar` doesn’t always compute all fields when you make changes to its values. I was thinking that this might be related to the behavior you observe, but then again not since a call to `getTimeInMillis()` should compute the fields. See for example [Setting values of Java Calendar does not give expected date-time](https://stackoverflow.com/questions/20981963/setting-values-of-java-calendar-does-not-give-expected-date-time). – Ole V.V. Jul 23 '17 at 03:23
  • 2
    Can we rule out a concurrency issue? `Calendar` is not thread safe. – Ole V.V. Jul 23 '17 at 03:26
  • 1
    It may not be related to your issue: The `h` you get in `onTimeChanged` is hour of day on a 24 hour clock, so you want `calendar.set(Calendar.HOUR_OF_DAY, h);`. – Ole V.V. Jul 23 '17 at 03:47
  • Changed `HOUR` to `HOUR_OF_DAY` and so far I can spin the hour and minute spinners to my heart's content and the date fields are rock steady. Please post above comment as an answer. – Lori Jul 23 '17 at 04:17

1 Answers1

5

This line is wrong:

            calendar.set(Calendar.HOUR, h);

Instead you want:

            calendar.set(Calendar.HOUR_OF_DAY, h);

The h you get in onTimeChanged() is hour of day on a 24 hour clock. Calendar.HOUR means hour within AM or PM, that is, on a 12 hour clock.

How could this cause your issue? Example: Original time is 20:25 (or 8:25 PM if you like). User changes it to 13:28 (1:28 PM). You set Calendar.HOUR to 13. Since the original time was in PM and you don’t change this, you are effectively setting the time to 13:28 PM. This is nonsense, but Calendar doesn’t care, it just interprets it as 1:28 AM the next day. Setting the day doesn’t help, because at the time you do that, fields are not yet computed, so internally the time is still 13:28 PM on the correct day. Only when in the end you call getTimeInMillis(), are all the fields computed, the overflow detected and the date incremented.

Calendar is default lenient

If you want to be notified about such an error (one that we all make from time to time), call setLenient(false) on your Calendar object after creating it. This will cause it not to accept out-of-range values like 13 for HOUR.

java.time

If you don’t want to bother with the Calendar class at all, you’re certainly far from the first one, and there is a better and more programmer friendly alternative. The modern Java date and time API is built into Java 8 and later, and on Android you can have it in the ThreeTenABP, see How to use ThreeTenABP in Android Project. I would suggest keeping the date in a LocalDate object and the time of day in a LocalTime so the user may update each independently with no risk of cross-contamination. Only in updateDropTime() would you combine the two into a LocalDateTime using LocalDate.atTime(), and then format it into your desired display format using a DateTimeFormatter.

public void onTimeChanged(TimePicker tp, int h, int m) {
    System.out.println("onTimeChanged(tp," + h + "," + m + ")");
    time = LocalTime.of(h, m);
    updateDropTime();
}

Updating could go like:

private void updateDropTime() {
    String disp = date.atTime(time)
            .format(DateTimeFormatter.ofLocalizedDateTime(FormatStyle.SHORT));
    dropTime.setText(disp);
}
Ole V.V.
  • 81,772
  • 15
  • 137
  • 161