1
\$\begingroup\$

This method add all numbers in list and return it as double

public HashMap<String, Double> add(HashMap<String, List<? extends Number>> map){
    HashMap<String, Double> result = new HashMap<>();

    map.forEach((key, value) -> {
        Double sum = 0.0;

        for (Number num : value){
            sum += num.doubleValue();
        }

        result.put(key, sum);
    });
    return result;
}

Testing my method with different types

public static void main(String[] args) {
    Adder adder  = new Adder();
    HashMap<String, List<? extends Number>> hashMap = new HashMap<>();

    List<Integer> list1 = new ArrayList<>();
    list1.add(1);
    list1.add(2);
    list1.add(3);

    List<BigDecimal> list2 = new ArrayList<>();
    list2.add(new BigDecimal("9.87654321e300"));
    list2.add(new BigDecimal("987654321"));
    list2.add(new BigDecimal("987654321"));


    hashMap.put("list1", list1);
    hashMap.put("list2", list2);

    System.out.println(adder.add(hashMap));


}

Is there way to improve this?

\$\endgroup\$
1
\$\begingroup\$

The following code is very inefficient:

    Double sum = 0.0;

    for (Number num : value){
        sum += num.doubleValue();
    }

It creates a Double object for 0.0, and then, since Double objects are immutable, for every num in the value list, it creates yet another Double object for the resulting partial sum. If you replaced Double with double, then you won't be creating new objects for each iteration through the loop.


Since you are already using the stream API, why not use DoubleStream::sum to sum the values as a double?

    double sum = value.stream().mapToDouble(Number::doubleValue).sum();

Finally, you could use the Collectors.toMap() stream method to create the map.

public Map<String, Double> add(Map<String, List<? extends Number>> map) {
    return map.entrySet().stream()
          .collect(Collectors.toMap(Map.Entry::getKey,
                                    e -> e.getValue().stream()
                                              .mapToDouble(Number::doubleValue)
                                              .sum()));
}

Whether or not this last step is an "improvement" may be a matter of taste.

Note: the return type is no longer a HashMap; it just a Map.


The add() method doesn't depend on any members of Adder, so the method should be static. Then you can get rid of the Adder adder = new Adder() object.


Using List.of(...) and Map.of(...) can simplify your test code:

public static void main(String[] args) {

    List<Integer> list1 = List.of(1, 2, 3);
    List<BigDecimal> list2 = List.of(new BigDecimal("9.87654321e300"),
                                     new BigDecimal("987654321"),
                                     new BigDecimal("987654321"));

    Map<String, List<? extends Number>> hashMap = Map.of("list1", list1,
                                                         "list2", list2);

    System.out.println(Adder.add(hashMap));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.