Math.min Military Time vs Base 8

Question

I'm making a program designed to get the elapsed time via military time and this is what I got so far:

public class TimeInterval {
    private int firstTime;
    private int secondTime;

    public TimeInterval(int _first,int _second) {
        if (_first < 0 || _second < 0) {
            System.out.println("ERROR INVALID INPUT");
            System.exit(0);
        }
        else if (_first > 2400 || _second > 2400) {
            System.out.println("ERROR INVALID INPUT");
            System.exit(0); 
        }
        else
            firstTime = Math.max(_first, _second);
            secondTime = Math.min(_first,_second);
        }
    }

    public int getHours() {
        return (Integer.parseInt(Integer.toString(firstTime).substring(0, 2)))-(Integer.parseInt(Integer.toString(secondTime).substring(0, 2)));
    }
}

The program works for the most part except when dealing with inputs of _first or _second that are from 0000 to 0700. The idea is that they are read as military time, but java reads them as base 8 integers and so when _first = 0700 and _second = 1400 I get 448 or something. Is there anyway I can ensure that when _first and _second are entered into Math.min they are read as base 10 instead of base 8.

Why are you storing time as `int`? You should store time as `LocalTime`. — Sweeper, Oct 03 '20 at 05:50
The problem is not in `Math.min`. The problem isn't even in this code. The problem is **most likely** in the way that you are testing this code. A Java integer literal `0700` actually *means* 448. The leading zero makes it an octal literal. Note that this only happens for literals, not for strings that you convert to integers using (for example) `Integer.parseInt`. — Stephen C, Oct 03 '20 at 06:55
But the other thing to note is that "0700" is not conceptually an integer. It is actually "07" hours + "00" minutes; i.e. two integers. So the conceptually correct way to represent it is as a Java String; i.e. `"0700"`. — Stephen C, Oct 03 '20 at 06:58
@StephenC Or even more correct, two integers. And then you may just as well take the step to `LocalTime` from java.time. — Ole V.V., Oct 03 '20 at 07:00

Ole V.V. · Answer 1 · 2020-10-03T07:03:17.590

It seems you are assuming that military hours are always 4 digits with an appropriate number of leading zeroes, from 0000 through 2359. You can’t represent this concept of military hours in an int since an int hasn’t necessarily got 4 digits. The same number can be formatted into 6, 06, 006, 0006 or 000000006. Internally it’s just the same 32 bits. So when firstTime is 700 (or 0700), then Integer.toString(firstTime).substring(0, 2)) takes the first two digits of 700 and yields 70 (not 7). So new TimeInterval(700, 1400).getHours() yields -56 (not 7). If firstTime is 9 (0009) or less, your code will probably crash with a StringIndexOutOfBoundsException.

A solution would be to pass your military hours in a String that always has length 4. Then your substring operation will always take the first 2 digits, the hours. If you insist on fighting your way through with int, another solution would be a modulo 100 operation to get the hours: firstTime % 100.

You mentioned base 8, also known as octal. There is nothing in the code you have shown that would cause base 8 to be used. Of course, if you use your class like new TimeInterval(0700, 1400), then Java will take 0700 to be base 8, so 448, you are correct. In this case the hours you get will be 14 - 44 = -30. Again passing a string will solve it.

Digging a bit deeper and finding a good solution

I would further like to suggest:

When the times are 1059 and 1202, there are 1 hour 3 minutes between them. Don’t you want the hours to be 1 rather then 2 in this case?
Use the LocalTime class of java.time, the modern Java date and time API, for your time of day. When you use military hours in some interface, keep them in the interface, no matter if in String or int form or both. Do the proper conversion when building your TimeInterval instance.

So your class may become:

public class TimeInterval{
    private static final DateTimeFormatter FORMATTER_FOR_MILITARY_HOURS
            = DateTimeFormatter.ofPattern("HHmm");
    
    private LocalTime firstTime;
    private LocalTime secondTime;

    /** Main constructor */
    public TimeInterval(LocalTime firstTime, LocalTime secondTime){
        if (firstTime.isAfter(secondTime)) {
            // Switch around
            this.firstTime = secondTime;
            this.secondTime = firstTime;
        } else {
            this.firstTime = firstTime;
            this.secondTime = secondTime;
        }
    }

    /** Convenience constructor accepting military hours */
    public TimeInterval(String first, String last) {
        this(LocalTime.parse(first, FORMATTER_FOR_MILITARY_HOURS),
                LocalTime.parse(last, FORMATTER_FOR_MILITARY_HOURS));
    }

    public int getHours(){
        long differenceInHours = ChronoUnit.HOURS.between(firstTime, secondTime);
        return Math.toIntExact(differenceInHours);
    }

}

This also gives us a range check for free: LocalTime only handles time from 00:00 to 23:59:59.999999999. So passing time soutside this range will throw an exception (sorry, 2400 cannot be handled). The same will happen if the string is not in the correct format, like when the length isn’t 4 or the minutes are greater than 59.

The this(…) in the convenience constructor is a call to the other constructor, the one accepting LocalTIme as arguments.

ChronoUnit.between() returns a long. Since we know that there can be 23 hours at most, we can safely convert to int. Math.toIntExact() does that for us.

Math.min Military Time vs Base 8

1 Answers1

Digging a bit deeper and finding a good solution