Yet another bug
I came across another silly little bug today. Take a look at the code below (assume ‘days’ is a parameter and that this snippet is part of a larger function)
Date now = new Date(); long nowMillis = now.getTime(); Timestamp nowTimestamp = new Timestamp(nowMillis); long future = 3600 * 24 * days * 1000; Timestamp expiryTimestamp = new Timestamp(nowMillis + future); System.out.println(nowTimestamp); System.out.println(expiryTimestamp);
Can you tell what is wrong with it ? What the code intends to do is to set an expiry timestamp to an element – X days from today. A caller called this code snippet with the value 5. Well there is nothing wrong so far. Here is the output
2010-04-27 10:37:10.497
2010-05-02 10:37:10.497
So then came along an element that needed an expiry of around 40 days. The caller calls with the value 40. Guess what the output is
2010-04-27 10:38:43.372
2010-04-17 17:35:56.076
Umm… so the expiry date is *before* the current date. Super. So what is going wrong ? The value of future in this case is -838967296. Although future is a long type, the data type that is being calculated to assign a value to it, is int. An int cannot take anything more than Integer.MAX_VALUE (2147483647).
If future had been calculated correctly, its value would be 3456000000. Instead what is being presented is the wrapped around value obtained after adding the remainder to Integer.MIN_VALUE.
I guess that just goes to show that if it aint broke you may still need to fix it later. Of course that depends on what your definition of ‘broke’ is 
A way to solve dates math is with calendar more or less like this
Date date = new Date(); // received as parameter
int days = 5; //
Calendar expiry = Calendar.getIntance();
expiry.setTime(date);
expiry.add(Calendar.DATE,days);
return new Timestamp(expiry.getTimeInMillis());
Here is the bug:
long future = 3600 * 24 * days * 1000;
Simple fix would be:
long future = 3600 * 24 * days * 1000L;
so it’s this part?: 3600 * 24 * days * 1000
where each factor is an integer
Can you tell what is wrong with it?
Yes, you are not using Joda Time http://joda-time.sourceforge.net/ for your date and time calculations
no bug.
but if your parameter days is a int the calculation is executed as a (correct) multiplication of 4 int’s and the result is put in a variable of type long.
if you want to do the calculation as long’s at least one of the numbers should be of the type long.
so long future = 3600l * 24 * days * 1000; will do the job as you wish
Why not just write it this way:
long future = 3600L * 24L * days * 1000L;
Actually, this is not a bug, but a little bit the fault of owned developer. Even C has this issue.
The developer assumes that
long future = 3600 * 24 * days * 1000;
will compute as long, but that is not true. Because every component of “3600 * 24 * days * 1000″ is int – the result is int, too. The type of “future” does not matter (because it is irrelevant for the computing part).
If you want a calculating “on long level” at least one component must be long. If this is not so, you must at least one of them. Option 1 use no “int days” but “long days” or option 2 cast one of the factors, like “3600L”.
The issue, other story:
float d = 1/2
The result is not 0.5 because int/int has no decimals.
Anyway, the usage of calendar and “real” 40 days would be better. The next 40 days can be 3456000001 seconds, too.
@Anonymous Coward
Never looked at JODA time. Ah one more java lib out there
@Nov
@steven
@DD605
@Michal R.
Yep that is a solution one could use. If one of the participants in the operations is a long, the result would have to be a long.
@knalli
@Fabricio Tuosto
Yep I agree that a calendar could be used here to provide a better solution.
When I said bug, I meant it was a bug from the method and developer’s perspective. This component is supposed to return the expiry time but it does the opposite sometimes
Hi,
using the calendar is tricky too, I wrote a post featuring a few calendar related puzzlers:
http://javarizon.wordpress.com/2010/04/25/java-calendar-puzzlers/
This bug cannot exist in Ada because, in Ada, integers do not overflow; instead they raise an exception (Constraint_Error, “overflow check failed”) that allows you to find the problem immediately. Combine this with the standard library types for manipulating dates, which ensure you cannot *possibly* have e.g. month numbers outside the range 1 ..12, etc:
package Standard is
[...]
type Duration is delta implementation-defined range implementation-defined;
end Standard;
(Duration is a fixed-point type representing seconds with absolute accuracy, as opposed to a floating-point type that has relative accuracy).
package Ada.Calendar is
type Time is private;
subtype Year_Number is Integer range 1901 .. 2399;
subtype Month_Number is Integer range 1 .. 12;
subtype Day_Number is Integer range 1 .. 31;
subtype Day_Duration is Duration range 0.0 .. 86_400.0;
function Clock return Time;
function Year (Date : Time) return Year_Number;
function Month (Date : Time) return Month_Number;
function Day (Date : Time) return Day_Number;
function Seconds(Date : Time) return Day_Duration;
procedure Split (Date : in Time;
Year : out Year_Number;
Month : out Month_Number;
Day : out Day_Number;
Seconds : out Day_Duration);
function Time_Of(Year : Year_Number;
Month : Month_Number;
Day : Day_Number;
Seconds : Day_Duration := 0.0)
return Time;
function “+” (Left : Time; Right : Duration) return Time;
function “+” (Left : Duration; Right : Time) return Time;
function “-” (Left : Time; Right : Duration) return Time;
function “-” (Left : Time; Right : Time) return Duration;
function “<" (Left, Right : Time) return Boolean;
function "” (Left, Right : Time) return Boolean;
function “>=”(Left, Right : Time) return Boolean;
Time_Error : exception;
private
… — not specified by the language
end Ada.Calendar;
Ada programmers consequently spend comparatively very little time debugging their software.
@Ludovicc Brenta
I do not know Ada, but you raise a good point. Thanks for sharing that.
On a side note, this bug can actually be exploited in certain cases.