patternjavaMinor
Calculate sum of all the items in array list?
Viewed 0 times
theallarrayitemscalculatesumlist
Problem
I am setting the sum total of all the items in an array to a text view in android.The values contained in the array are all currency values.
So I am basically calculating the sum total and the vat which is 2%.
The code is pretty straightforward, but I am interested in any possible improvement.
@Override
public void afterTextChanged(Editable s) {
int id = 0;
id = (int) itemtotal.getTag();
String itemTot = itemtotal.getText().toString();
if (id >= 0 && id <= totals.size()) {
totals.set(id - 1, itemTot);
double[] doubleList = new double[totals.size()];
double newsum = 0.0d;
for (int i = 0; i < totals.size(); ++i) {
doubleList[i] = Double.parseDouble(totals.get(i));
newsum += (double) Math.round((doubleList[i]) * 100.0) / 100.0;
}
textViewSum.setText(((double) Math.round((newsum * (0.02) + newsum) * 100.0) / 100.0) + "");//set total text to sum
textViewVat.setText(((double) Math.round((newsum * (0.02)) * 100.0) / 100.0) + "");
}
}So I am basically calculating the sum total and the vat which is 2%.
The code is pretty straightforward, but I am interested in any possible improvement.
Solution
Naming
Avoid abbreviations.
Use camelCase.
It's confusing that
Implementation
Don't declare
Guard clauses are very helpful for keeping code from looking like a big > sign. Not an issue now, but it might be if more code gets added to this method.
Don't declare
You don't need an entire
It might be nice if there were parens to clarify the cast is before the division. Also, you don't need to cast to double. Dividing by 100.0 makes the result a double even though the dividend is a
It's ugly to use string concatenation to turn a double into a String. Java provides a method (
Result
If you were to apply all these suggestions your code might look something like:
s is a bad parameter name. If there's nothing better, use editable.Avoid abbreviations.
itemTot might be better as itemTotalValue. Use camelCase.
newsum should be newSum.It's confusing that
textViewSum doesn't actually hold the sum.Implementation
Don't declare
id as 0 and then reassign it right away. Just declare it as what it should be, and make it final because it won't be reassigned.Guard clauses are very helpful for keeping code from looking like a big > sign. Not an issue now, but it might be if more code gets added to this method.
Don't declare
itemtot until it's actually needed. You might leave the method before using the value.You don't need an entire
double[] to hold your computations. You only need one double variable declared inside the for loop.It might be nice if there were parens to clarify the cast is before the division. Also, you don't need to cast to double. Dividing by 100.0 makes the result a double even though the dividend is a
long.It's ugly to use string concatenation to turn a double into a String. Java provides a method (
Double.toString()) for simple conversions, and a class (NumberFormat) for complex conversions. Or you can use String.format(), which is a bit newfangled for me, but I hear the kids love it.Result
If you were to apply all these suggestions your code might look something like:
@Override
public void afterTextChanged(final Editable editable) {
final int id = (int) itemtotal.getTag();
if ((id totals.size())) {
return;
}
final String itemTotalValue = itemtotal.getText().toString();
totals.set(id - 1, itemTotalValue);
double newSum = 0.0d;
for (int i = 0; i < totals.size(); ++i) {
final double value = Double.parseDouble(totals.get(i));
newSum += Math.round(value * 100) / 100.0;
}
final double sum = Math.round((newSum * 0.02 + newSum) * 100.0) / 100.0;
textViewSum.setText(Double.toString(sum));
final double vat = Math.round((newSum * 0.02) * 100.0) / 100.0;
textViewVat.setText(Double.toString(vat));
}Code Snippets
@Override
public void afterTextChanged(final Editable editable) {
final int id = (int) itemtotal.getTag();
if ((id < 0) || (id > totals.size())) {
return;
}
final String itemTotalValue = itemtotal.getText().toString();
totals.set(id - 1, itemTotalValue);
double newSum = 0.0d;
for (int i = 0; i < totals.size(); ++i) {
final double value = Double.parseDouble(totals.get(i));
newSum += Math.round(value * 100) / 100.0;
}
final double sum = Math.round((newSum * 0.02 + newSum) * 100.0) / 100.0;
textViewSum.setText(Double.toString(sum));
final double vat = Math.round((newSum * 0.02) * 100.0) / 100.0;
textViewVat.setText(Double.toString(vat));
}Context
StackExchange Code Review Q#112177, answer score: 6
Revisions (0)
No revisions yet.