Scott Gray wrote:
> On 26/03/2010, at 6:32 PM, Adam Heath wrote:
> 
>> Scott Gray wrote:
>>> On 26/03/2010, at 5:13 PM, Adam Heath wrote:
>>>
>>>> The above method makes use of a BigDecimal.  This is wrong.  It should
>>>> be an int.  Originally, before the BigDecimal conversion, it was a
>>>> Double.  This method should have never been converted.
>>> It looked like a good candidate at the time and looking at it now, it still 
>>> looks like a good candidate for BigDecimal.  Why should it never have been 
>>> converted?
>> Days is a whole number.  2 billion days is over 5.8 million years.
>> Why would it need to be a Double previously, and a BigDecimal now?
> 
> Days is not a whole number, 1.5 days is perfectly valid.  I have no idea why 
> it needed to be a double previously but when you're converting thousands of 
> doubles to BigDecimals then you don't dwell on each one for too long.

I discovered this bug in an old system(2 years), that did repeat
billing using a shoppinglist that get's converted to a real
shoppingcart and order once a month.  I had a promo that would fire
after the activate the first time, giving the user one month free, and
 had a condition that made it valid for the first 28 days only.  This
failed this year, at this time, because feb 16 to mar 16 is actually
27 days, and 23 hours, which got rounded down to 27.

In the version I have, it is still a Double, but the algo doesn't do
an explicit cast, so integer arthmetic is being done, so it gets
rounded down.

In any event, this method should not have been a Double in the first
place, and I'm saying it both needs to be converted to an int, and the
calculation fixed.

Reply via email to