patternjavaMinor
Optimize parsing of number for currency conversion
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:
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:
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:
formatis not used.
exchangeCurrencyIDis 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.