I'm trying to use the below code to calculate the average of a set of values that a user enters and display it in a jTextArea
but it does not work properly. Say, a user enters 7, 4, and 5, the program displays 1 as the average when it should display 5.3
ArrayList <Integer> marks = new ArrayList();
Collections.addAll(marks, (Integer.parseInt(markInput.getText())));
private void analyzeButtonActionPerformed(java.awt.event.ActionEvent evt) {
analyzeTextArea.setText("Class average:" + calculateAverage(marks));
}
private int calculateAverage(List <Integer> marks) {
int sum = 0;
for (int i=0; i< marks.size(); i++) {
sum += i;
}
return sum / marks.size();
}
What is wrong with the code?
Correct and fast way compute average for List<Integer>
:
private double calculateAverage(List<Integer> marks) {
long sum = 0;
for (Integer mark : marks) {
sum += mark;
}
return marks.isEmpty()? 0: 1.0*sum/marks.size();
}
This solution take into account:
It works coorectly for List, because any list contains less that 2^31 int, and it is possible to use long as accumulator.
PS
Actually foreach allocate memory - you should use old style for() cycle in mission critical parts
List.stream().mapToDouble(a->a).average()
If using Java8 you can get the average of the values from a List as follows:
List<Integer> intList = Arrays.asList(1,2,2,3,1,5);
Double average = intList.stream().mapToInt(val -> val).average().orElse(0.0);
This has the advantage of having no moving parts. It can be easily adapted to work with a List of other types of object by changing the map method call.
For example with Doubles:
List<Double> dblList = Arrays.asList(1.1,2.1,2.2,3.1,1.5,5.3);
Double average = dblList.stream().mapToDouble(val -> val).average().orElse(0.0);
NB. mapToDouble is required because it returns a DoubleStream which has an average
method, while using map
does not.
or BigDecimals:
@Test
public void bigDecimalListAveragedCorrectly() {
List<BigDecimal> bdList = Arrays.asList(valueOf(1.1),valueOf(2.1),valueOf(2.2),valueOf(3.1),valueOf(1.5),valueOf(5.3));
Double average = bdList.stream().mapToDouble(BigDecimal::doubleValue).average().orElse(0.0);
assertEquals(2.55, average, 0.000001);
}
using orElse(0.0)
removes problems with the Optional object returned from the average
being 'not present'.
Using Guava, it gets syntactically simplified:
Stats.meanOf(numericList);
sum += i;
You're adding the index; you should be adding the actual item in the ArrayList
:
sum += marks.get(i);
Also, to ensure the return value isn't truncated, force one operand to double
and change your method signature to double
:
return (double)sum / marks.size();
When the number is not big, everything seems just right. But if it isn't, great caution is required to achieve correctness.
Take double as an example:
If it is not big, as others mentioned you can just try this simply:
doubles.stream().mapToDouble(d -> d).average().orElse(0.0);
However, if it's out of your control and quite big, you have to turn to BigDecimal as follows (methods in the old answers using BigDecimal actually are wrong).
doubles.stream().map(BigDecimal::valueOf).reduce(BigDecimal.ZERO, BigDecimal::add)
.divide(BigDecimal.valueOf(doubles.size())).doubleValue();
Enclose the tests I carried out to demonstrate my point:
@Test
public void testAvgDouble() {
assertEquals(5.0, getAvgBasic(Stream.of(2.0, 4.0, 6.0, 8.0)), 1E-5);
List<Double> doubleList = new ArrayList<>(Arrays.asList(Math.pow(10, 308), Math.pow(10, 308), Math.pow(10, 308), Math.pow(10, 308)));
// Double.MAX_VALUE = 1.7976931348623157e+308
BigDecimal doubleSum = BigDecimal.ZERO;
for (Double d : doubleList) {
doubleSum = doubleSum.add(new BigDecimal(d.toString()));
}
out.println(doubleSum.divide(valueOf(doubleList.size())).doubleValue());
out.println(getAvgUsingRealBigDecimal(doubleList.stream()));
out.println(getAvgBasic(doubleList.stream()));
out.println(getAvgUsingFakeBigDecimal(doubleList.stream()));
}
private double getAvgBasic(Stream<Double> doubleStream) {
return doubleStream.mapToDouble(d -> d).average().orElse(0.0);
}
private double getAvgUsingFakeBigDecimal(Stream<Double> doubleStream) {
return doubleStream.map(BigDecimal::valueOf)
.collect(Collectors.averagingDouble(BigDecimal::doubleValue));
}
private double getAvgUsingRealBigDecimal(Stream<Double> doubleStream) {
List<Double> doubles = doubleStream.collect(Collectors.toList());
return doubles.stream().map(BigDecimal::valueOf).reduce(BigDecimal.ZERO, BigDecimal::add)
.divide(valueOf(doubles.size()), BigDecimal.ROUND_DOWN).doubleValue();
}
As for Integer
or Long
, correspondingly you can use BigInteger
similarly.
You can use standard looping constructs or iterator/listiterator for the same :
List<Integer> list = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8);
double sum = 0;
Iterator<Integer> iter1 = list.iterator();
while (iter1.hasNext()) {
sum += iter1.next();
}
double average = sum / list.size();
System.out.println("Average = " + average);
If using Java 8, you could use Stream or IntSream operations for the same :
OptionalDouble avg = list.stream().mapToInt(Integer::intValue).average();
System.out.println("Average = " + avg.getAsDouble());
Reference : Calculating average of arraylist
Use a double for the sum, otherwise you are doing an integer division and you won't get any decimals:
private double calculateAverage(List <Integer> marks) {
if (marks == null || marks.isEmpty()) {
return 0;
}
double sum = 0;
for (Integer mark : marks) {
sum += mark;
}
return sum / marks.size();
}
or using the Java 8 stream API:
return marks.stream().mapToInt(i -> i).average().orElse(0);
With Java 8 it is a bit easier:
OptionalDouble average = marks
.stream()
.mapToDouble(a -> a)
.average();
Thus your average value is average.getAsDouble()
return average.isPresent() ? average.getAsDouble() : 0;
Here a version which uses BigDecimal
instead of double
:
public static BigDecimal calculateAverage(final List<Integer> values) {
int sum = 0;
if (!values.isEmpty()) {
for (final Integer v : values) {
sum += v;
}
return new BigDecimal(sum).divide(new BigDecimal(values.size()), 2, RoundingMode.HALF_UP);
}
return BigDecimal.ZERO;
}
Source: Stackoverflow.com