71

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?

giannis christofakis
  • 8,201
  • 4
  • 54
  • 65
user1419306
  • 779
  • 2
  • 6
  • 10

11 Answers11

104

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; 
Mariana
  • 1,199
  • 1
  • 7
  • 6
  • 28
    `average.isPresent() ? average.getAsDouble() : defaultValue` can be simplified further to `optional.orElse( defaultValue )` – Oleg Estekhin Aug 21 '15 at 07:55
  • @OlegEstekhin - Shouldn't we be using mapToInt instead of mapToDouble ? Actually is mapping needed ? – MasterJoe Sep 25 '19 at 00:39
84

Why use a clumsy for loop with an index when you have the enhanced for loop?

private double calculateAverage(List <Integer> marks) {
  Integer sum = 0;
  if(!marks.isEmpty()) {
    for (Integer mark : marks) {
        sum += mark;
    }
    return sum.doubleValue() / marks.size();
  }
  return sum;
}

Update: As several others have already pointed out, this becomes much simpler using Streams with Java 8 and up:

private double calculateAverage(List <Integer> marks) {
    return marks.stream()
                .mapToDouble(d -> d)
                .average()
                .orElse(0.0)
}
Jeshurun
  • 22,940
  • 6
  • 79
  • 92
  • 5
    I'd check if marks.size() == 0 in the beggining, as this will divide by zero if the list if empty – Axarydax Feb 07 '13 at 06:11
  • 6
    I love java, but you gotta miss C#'s list.Average() function when you're doing this :p – John Humphreys Apr 23 '14 at 20:28
  • Just a quick note, one reason to use the clumsy loop is that it is a lot faster than the so-called civilized loop. For ArrayLists, the for(int i = 0 .... ) loop is about 2x faster than using the iterator or the for (:) approach, so even though it's prettier, it's a lot slower! One tip to make it go even faster is to cache the length as follows: for (int i = 0, len = list.size(); i – Leo Mar 08 '16 at 18:33
  • 1
    they're actually about as fast in properly done test. interestingly enough the "enhanced for loop" and the traditional for loop ends up being executed just as fast as a while(i-->0) one, despite having an extra evaluation/call per loop. this just running on se1.7, with the arraylist filled with objects having a random int as member variable and calculating that to a sum, to make the vm do actual work. enhanced loop is about as fast as iterating yourself with iterator. if you're using arraylist, no point in using the enhanced since index based gets are faster and less gc causing. – Lassi Kinnunen May 01 '16 at 19:12
  • A bit off-topic here but I was using this method in Android but Android Studio told me that the for loop required an Object type like `for(Object mark: marks)` (I really don't know why) obviously comes out another error inside the loop _"Operator '+' cannot be applied to 'java.lang.Double', 'java.lang.Object'"_ so I had to cast `mark` to Double: `sum += (Double)mark;` – Cliff Burton Sep 08 '16 at 17:52
  • Instead of [changing `sum` to `Integer`](https://stackoverflow.com/revisions/10791597/2), effectively boxing every intermediate value, it would have been more sensible to change `sum.doubleValue()` to `(double)sum`, or just declare `sum` as `double` in the first place. Performing the summing with `double` still would be more efficient than boxing every value to `Integer`. – Holger Nov 20 '19 at 11:54
  • Why do you need to mapToDouble? Can use `Arrays.stream(marks).average().orElse(0.0);`. https://www.baeldung.com/java-array-sum-average – Shanika Ediriweera Jun 19 '22 at 03:25
43

From Java8 onward 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'.

robjwilkins
  • 5,462
  • 5
  • 43
  • 59
  • oops - never noticed the Java8 answer above which is the same as the one I gave – robjwilkins Aug 21 '15 at 07:51
  • 1
    in example 2, why is mapToDouble needed when the dblList contains Doubles? – simpleuser Jul 14 '17 at 21:11
  • 1
    @simpleuser - because mapToDouble returns a DoubleStream, which has an `average` method. – robjwilkins Apr 30 '18 at 10:09
  • I **do not** think the third method is working (using `mapToDouble(BigDecimal::doubleValue).average()`). You should use `BigDecimal::valueOf` instead. – Hearen Jun 26 '18 at 01:11
  • And actually even that, you are still wrong, since **average** is only working for **primitive types**. – Hearen Jun 26 '18 at 01:17
  • @Hearen - not sure why you think it doesn't work? I have updated the example with a straight forward junit example that you can run in your IDE to verify the code works. Perhaps you could post a new question if you don't understand my answer. cheers. – robjwilkins Jun 26 '18 at 10:02
  • @robjwilkins Try this `Arrays.asList(valueOf(Math.pow(10, 308)),valueOf(Math.pow(10, 308)), valueOf(Math.pow(10, 308)),valueOf(Math.pow(10, 308)));` – Hearen Jun 26 '18 at 11:49
  • You can test this list with my **answer** I posted hours ago. – Hearen Jun 26 '18 at 11:49
  • Besides I thought you were using `List` to keep consistency and in your latest version you are using `List` which is actually will provide a quite different interface breaking the contract with the interface users. – Hearen Jun 26 '18 at 11:50
  • The problem in your solution is this `If any recorded value is a NaN or the sum is at any point a NaN then the average will be NaN.` You are directly using `average()` for the `BigDecimal` which actually make it **downgraded** to the double. – Hearen Jun 26 '18 at 11:55
18

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);
Emmanuel Bourg
  • 9,601
  • 3
  • 48
  • 76
11
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();
Ry-
  • 218,210
  • 55
  • 464
  • 476
7

Using Guava, it gets syntactically simplified:

Stats.meanOf(numericList);
Sayan Pal
  • 4,768
  • 5
  • 43
  • 82
  • If you can afford to have Guava in your project, this is SOOO much better than the ugly and verbose `.stream().mapToDouble(v -> v).average()` garbage! Thank you – higuaro Aug 31 '23 at 23:50
2
List.stream().mapToDouble(a->a).average()
Hearen
  • 7,420
  • 4
  • 53
  • 63
2

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.

Hearen
  • 7,420
  • 4
  • 53
  • 63
1

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:

  • Handle overflow
  • Do not allocate memory like Java8 stream
  • Do not use slow BigDecimal

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

sibnick
  • 3,995
  • 20
  • 20
1

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

Sekhar Ray
  • 31
  • 1
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;
}
yglodt
  • 13,807
  • 14
  • 91
  • 127