HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Optimize parsing of number for currency conversion

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conversionnumberparsingforoptimizecurrency

Problem

I have written this working block, but is it optimized? I am getting currency in this format: 1.234,25.

public static String convertCurrency(String value, String exchangeCurrencyId)
{
    java.text.NumberFormat format = null;
    value = value.replaceAll("\\.", ",");
    char[] chars = value.toCharArray();
    chars[value.lastIndexOf(',')] = '.';
    value = new String(chars);
    value = value.replaceAll(",", "");
    Double price = Double.valueOf(value);
    price = price * (50); //exchange rate

    return price.toString();
}

Solution

This code has various issues. Here is what I understand your code does:


The code takes a formatted number which consists of digits, separators (commas or periods). The last separator is used as the decimal separator, all other separators are deleted.


Then it multiplies the number with some exchange rate, and returns that as a string.

The issues with your code are:

  • format is not used.



  • exchangeCurrencyID is not used.



  • The exchange rate is hard-coded



  • You use regexes for simple transliterations



  • Your code obfuscates the intent.



  • Your method does multiple unrelated things:



  • Sanitizing and parsing the number string



  • Applying exchange rate conversion


These separate responsibilities should be performed by two separate methods.

If we wish to avoid using regexes for a task that does not need them, we could write something like this:

StringBuilder sanitized = new StringBuilder();
boolean decimalSeparatorFound = false;
char[] chars = value.toCharArray();
for (int i = chars.length-1; i >= 0; i--) {
    if (chars[i] == ',' || chars[i] == '.') {
        if (decimalSeparatorFound) continue;  // skip this char
        decimalSeparatorFound = true;
        sanitized.append('.');
    } else {
        sanitized.append(chars[i]);
    }
}
double price = Double.valueOf(sanitized.reverse().toString());

...

Code Snippets

StringBuilder sanitized = new StringBuilder();
boolean decimalSeparatorFound = false;
char[] chars = value.toCharArray();
for (int i = chars.length-1; i >= 0; i--) {
    if (chars[i] == ',' || chars[i] == '.') {
        if (decimalSeparatorFound) continue;  // skip this char
        decimalSeparatorFound = true;
        sanitized.append('.');
    } else {
        sanitized.append(chars[i]);
    }
}
double price = Double.valueOf(sanitized.reverse().toString());

...

Context

StackExchange Code Review Q#38317, answer score: 6

Revisions (0)

No revisions yet.