Adding time doesn't add up

Question

Sorry, but I come to you with a problem that should have been answered already. Alas, my google-foo is weak and I humbly come to ask for your guidance.

WHY DOES AN EXTRA 59 minutes appear??

Ok, so I got a "Clock In" thing going and my calculations go wrong when the persons out minutes are the same as the in minutes. I remember hearing about this problem before, but with no solution. Just a "Well, that's how it is. Good luck, brah." I've tried adding and subtracting minutes during calculation, but that just pushes the problem +/- the time added/subtracted. I've also tried calculating to the second (not shown below), but that also didn't help.

Obligatory, here's my code:

//calculateTotalHours() 
public static String calculateTotalHours(Cell inTime, Cell outTime, Cell breakStart, Cell breakEnd)
{
    System.out.println(LOG + "calculateTotalHours");
    SimpleDateFormat timeFormat = new SimpleDateFormat("HH:mm");
    Date in, out, start, end;       
    
    if(null != inTime && null != outTime && null != breakStart && null != breakEnd)
    {   
        try
        {
            in = timeFormat.parse(inTime.getStringCellValue());
            out = timeFormat.parse(outTime.getStringCellValue());
            start = timeFormat.parse(breakStart.getStringCellValue());
            end = timeFormat.parse(breakEnd.getStringCellValue());

            long lunchTotal = end.getTime() - start.getTime();
            long totalWork = out.getTime() - in.getTime();
            long totalTime = totalWork - lunchTotal;            
            long diffHours = totalTime/HOURS % 24;
            totalTime -= diffHours;
            long diffMinutes = totalTime/MINUTES % 60;
            totalTime -= diffMinutes;       
            
            return String.format("%02d:%02d", diffHours, diffMinutes);
        } 
        catch (ParseException e) {e.printStackTrace();}
    }
    else
    {
        try
        {
            if(null == breakStart || null == breakEnd)              {
                in = timeFormat.parse(inTime.getStringCellValue());
                out = timeFormat.parse(outTime.getStringCellValue());           
                
                long totalTime = out.getTime() - in.getTime();              
                long diffHours = totalTime/HOURS % 24;
                totalTime -= diffHours;
                long diffMinutes = totalTime/MINUTES % 60;
                totalTime -= diffMinutes;
                
                return String.format("%02d:%02d", diffHours, diffMinutes);
            }
            else if(null == inTime.getStringCellValue())
            {
                System.out.println(LOG + "inTime is blank");
                return "-1";
            }
        } 
        catch (ParseException e) {e.printStackTrace();}
    }
    return "-1";
}

My sincere apologies for the mess I call my code. And let me know if calculating by the second or millisecond is the way to go. I may have overlooked something when trying it that way.

2020 UPDATE:

I wanted to update my code and what I should have done. First, I should have separated the cells, strings, and time more. Second, I should have broken it down to more methods for clarity.

I'll just show getTotalHours(startTimeString, endTimeString):

    DateTimeFormatter format = DateTimeFormatter.ofPattern("HH:mm");
    LocalTime end = LocalTime.parse(endTimeString.trim(), format);
    LocalTime start = LocalTime.parse(startTimeString.trim(), format);
    
    return   Duration.between(start, end).toHoursPart()%24 
           + ":"
           + Duration.between(start, end).toMinutes()%60;

First lesson of handling time and dates: never, under any circumstances whatsoever, do calculations yourself. You would be assuming certain things that aren't true, such as that all minutes have 60 seconds. — SeverityOne, Apr 23 '18 at 08:51
Avoid `SimpleDateFormat` and its friends, it’s troublesome and they are long outdated. Use `Duration` from `java.time` (the modern Java date and time API) for handling amounts of time. — Ole V.V., Apr 23 '18 at 09:22
Your constants `HOURS` and `MINUTES`, how are they defined? Not that you ought to declare such constants yourself, just trying to understand what more exactly went wrong. — Ole V.V., Apr 23 '18 at 09:24
@SeverityOne How can I not assume all minutes have 60 seconds?? But yeah, I tried having my excel sheet do the calculation for me, but I read somewhere that it's better to have it done programmatically. I knew I left something out: int SECONDS = 1000; int MINUTES = 60*SECONDS; int HOURS = 60*MINUTES; And I'll be avoiding those APIs once I get a handle on the LocalTime API. — Francisco, Apr 23 '18 at 22:36
It's called a [leap second](https://en.wikipedia.org/wiki/Leap_second). You just never want to do any time calculations by hand, because there is a bewildering number of exceptions to consider. — SeverityOne, Apr 24 '18 at 05:01

score 2 · Answer 1 · answered Apr 23 '18 at 08:50

Your math is all wrong for dealing with the fact that the time values returned by getTime() are in milliseconds.

I'm guessing (since this isn't shown anywhere above) that HOURS and MINUTES are constants for the number of milliseconds in an hour and in a minute, respectively? If so, you're only partially dealing with the fact that the units are milliseconds, because when you do this:

totalTime -= diffHours;

...you'd be subtracting only a few milliseconds from your total time, not achieving what I think you think you're accomplishing, which is removing all of the hours so that what's left is minutes.

There are other problems with this code too, but that's the most glaring problem.

That's a huge oversight. Sorry about that. I thought I followed an example correctly, but checking it again, I see where it went wrong. Below is another code. @OleV.V showed an example on how to use LocalTime rather than SimpleDateFormat. Another user also recommended switching over to that API and LocalDateTime. I'll do that till I want to bash my head in a wall again. Thanks! — Francisco, Apr 23 '18 at 23:04

Ole V.V. · Accepted Answer · 2018-04-23T10:10:16.410

private static final String defaultHours = "-1";

public static String calculateTotalHours(Cell inTime, Cell outTime, Cell breakStart, Cell breakEnd)
{
    try {
        LocalTime inLocalTime = LocalTime.parse(inTime.getStringCellValue());
        LocalTime outLocalTime = LocalTime.parse(outTime.getStringCellValue());
        LocalTime breakStartTime = LocalTime.parse(breakStart.getStringCellValue());
        LocalTime breakEndTime = LocalTime.parse(breakEnd.getStringCellValue());

        Duration lunch = Duration.between(breakStartTime, breakEndTime);
        Duration present = Duration.between(inLocalTime, outLocalTime);
        Duration worked = present.minus(lunch);

        return String.format("%02d:%02d", worked.toHours(), worked.toMinutesPart());

    } catch (DateTimeParseException dtpe) {
        System.out.println(dtpe);
        return defaultHours;
    }
}

The above code uses the toMinutesPart method that was introduced in Duration in Java 9. Assuming you are using ThreeTenABP (details below) or an earlier Java version, it’s:

        long hours = worked.toHours();
        long minutes = worked.minusHours(hours).toMinutes();
        return String.format("%02d:%02d", hours, minutes);

For clarity I have omitted the null checks, I am sure you can handle those.

As said in the comments, leave time math to well-tested library methods. It’s not only less error-prone, it also gives clearer code. I am using java.time, the modern Java date and time API. It also saves us from specifying an explicit formatter since LocalTime.parse() parses your format of HH:mm as its default (it’s ISO 8601 format).

What went wrong in your code?

As kshetline in the other answer I believe that totalTime is in milliseconds and diffHours is hours. So when you do totalTime -= diffHours;, you subtract hours from milliseconds and you’re bound to get an incorrect result (same with totalTime -= diffMinutes;, by the way, but you’re not using the result of the latter subtraction, so this error doesn’t show).

Question: Can I use java.time on Android?

Yes, java.time works nicely on older and newer Android devices. It just requires at least Java 6.

In Java 8 and later and on newer Android devices (from API level 26, I’m told) the modern API comes built-in.
In Java 6 and 7 get the ThreeTen Backport, the backport of the new classes (ThreeTen for JSR 310; see the links at the bottom).
On (older) Android use the Android edition of ThreeTen Backport. It’s called ThreeTenABP. And make sure you import the date and time classes from org.threeten.bp with subpackages.

Links

Oracle tutorial: Date Time explaining how to use java.time.
Java Specification Request (JSR) 310, where java.time was first described.
ThreeTen Backport project, the backport of java.timeto Java 6 and 7 (ThreeTen for JSR-310).
ThreeTenABP, Android edition of ThreeTen Backport
Question: How to use ThreeTenABP in Android Project, with a very thorough explanation.
Wikipedia article: ISO 8601

Adding time doesn't add up

2 Answers2

What went wrong in your code?

Question: Can I use java.time on Android?

Links