0

I have a class (let's call it Money) that handles calculation of monetary values. It's a wrapper for BigDecimal and uses the correct decimal precision for money. It has worked pretty well for many years, but we have had an incident that had some major repercussions and need to determine the cause, and if the use of this class was the cause.

We read values from an Excel file using jExcelApi. We use this API because the files we are working with are Excel 2003 files. The value is read from this file using the getContents method. The field in the Excel file is in the General format and after reading it, the result instantiates a new instance of the money class using the following static method of the Money class:

public static Money valueOf(String s, int precision) {
    try {
        Number n = format.parse(s);
        return new Money(new BigDecimal(n.doubleValue()), precision);
    }
    catch (ParseException e) {
        throw new IllegalArgumentException("Cannot convert as money " + s);
    }
}

precision is 2 and format is an instance of DecimalFormat("#0.00###;-0.00###")

e.g.

Money amount = Money.valueOf(sheet.getCell(col, row).getContents());

So a value of 1500 (no decimals) was used in the sheet and used as input. What happened was that this value somehow got duplicated in the following way 15150000 creating a severely overstated amount. It's as if the same value was pushed in between the digits. So instead of only 1500, the value was 15 million.

I've have not been able to replicate this problem. The values are always calculated correctly. It's quite possible that the system was operating at peak at the time and that the swap space was being used, but I don't see how this would only affect one record of over 3500 records with the same amount.

Any ideas? Is it possible that the memory was not cleared?

Our technology stack:
Ubuntu 15.04 with 8GB RAM
Java 8
Tomcat 8.5
MySQL 5.7.15

Xenno
  • 46
  • 2
  • 1
    `BigDecimal` has a constructor, that takes a `String`, do not use the one that takes a `double`... – Usagi Miyamoto Aug 15 '17 at 09:11
  • 1
    Why do you even need the `BigDecimal` when you're using `Double` intermittenly when constructing it? There is literally no point in using it in your circumstances, it offers you no benefit over just using `Double` (other than you can _lie_ to your API users that you use high-precision arithmetics when you actually are not). Other than that, I strongly recommend reading `Javadoc` for all constructors of `BigDecimal` class - you're probably using the wrong one. – M. Prokhorov Aug 15 '17 at 09:21
  • Also, a note from the point of class names: a `Money` amount means nothing without unit of measure (the `Currency`), which your class doesn't seem to have. – M. Prokhorov Aug 15 '17 at 09:23
  • 2
    Sounds to me like you need a `ThreadLocal DecimalFormat`. `DecimalFormat` is **not** thread safe and this is exactly the kind of affect you would get under heavy load. – OldCurmudgeon Aug 15 '17 at 09:59
  • Thank you @OldCurmudgeon, I think you have answered my question. Would there be any way to replicate this scenario? – Xenno Aug 15 '17 at 11:17
  • Thank you @M.Prokhorov for your valuable comments. This is a class that has been used for ages, and you are correct that the use of `Double` should have been a better solution – Xenno Aug 15 '17 at 11:18
  • 1
    @Xenno, you could try to replicate non-thread-safe behavior of your date format by imitating peak load, where it would be constantly assaulted by requests to parse something under really heavy load. But this strategy isn't particularly stable, I don't think - you might get issue reproduced, or might not. – M. Prokhorov Aug 15 '17 at 11:37
  • 1
    @Xenno - Replicating threading issues is notoriously difficult. I would suggest you find a good source for a `ThreadLocal DecimalFormat`, implement it and hope the problem does not happen again. – OldCurmudgeon Aug 15 '17 at 12:36
  • Thanks @M.Prokhorov and OldCurmudgeon. Your assistance is highly appreciated. – Xenno Aug 16 '17 at 09:39
  • By the way. DecimalFormat has this nice setter called `setParseBigDecimal(boolean)`. You might want to look into it as well. – M. Prokhorov Aug 16 '17 at 12:21

0 Answers0