0

I have a class with a (simplified) method which returns a string like this :

private String doStuff(String currencyCode, BigDecimal amount){
    StringBuilder sb= new StringBuilder();
    NumberFormat format = NumberFormat.getInstance(Locale.FRANCE);
    Currency currency = Currency.getInstance(currencyCode);

    try{
        format.setCurrency(currency);
        sb.append(format.format(amount));
    }catch(Exception e){
        sb.append(amount);
        logger.error(e.getMessage());
    }
    return sb.toString();
}

However, we sometimes get a currencyCode which is not a supported ISO 4217 code, and Currency.getInstance(currencyCode); throws an IllegalArgumentException.

I've thought of 2 ways to deal with this :

  1. use a try/catch around the getInstance call
  2. check that the currencyCode is supported before

Option 1 :

private String doStuff(String currencyCode, BigDecimal amount){
    StringBuilder sb= new StringBuilder();
    NumberFormat format = NumberFormat.getInstance(Locale.FRANCE);
    Currency currency = null;

    try {
        currency = Currency.getInstance(currencyCode);
    } catch (IllegalArgumentException iae) {
        iae.printStackTrace();
    }
    ...
}

Option 2 :

private static Set<String> currencies = initCurrencies();

private static Set<String> initCurrencies() {
    Set<String> currencies = new HashSet<>();
    for (Currency c : Currency.getAvailableCurrencies()) {
        currencies.add(c.getCurrencyCode());
    }
    return currencies;
}

private String doStuff(String currencyCode, BigDecimal amount){
    StringBuilder sb= new StringBuilder();
    NumberFormat format = NumberFormat.getInstance(Locale.FRANCE);
    Currency currency = null;

    if (currencies.contains(currencyCode)) {
        currency = Currency.getInstance(currencyCode);
    }
    ...
}

Which is better ?

0

There's a third way to handle it, which you didn't mention

  1. Let the doStuff method throw the IllegalArgumentException (or a different type of exception, maybe InvalidCurrencyException) and let the calling method handle it (or re-throw it).

Which option you choose depends on what doStuff can do if it knows it has an invalid currency code. That is, in both of the options you've presented, what does the ... actually do with a null currency object? Does it throw a NullReferenceException later on? Does it return a string indicating the selected currency was invalid? Does it just return an empty string?

If you can do something meaningful with the null currency, then do so. If you can't, though, just let the exception propagate up (maybe wrapping it in a more descriptive exception), as that's what exceptions are supposed to do.

NB: Of the two options you presented, option 2 is better. If exceptions can be avoided, they should be (and printing out the stack trace to users should be avoided at all costs!), as exceptions are not flow control. In option 2, however, you need to consider what happens if the condition in the if statement returns false...

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.