CODEWITHSUNDEEP
javalistfor-looparraylistcollections
dingoKid
dingoKid

Reputation: 63

Adding two lists of own type

I have a simple User class with a String and an int property.

I would like to add two Lists of users this way:

Like this:

List1: { [a:2], [b:3] }
List2: { [b:4], [c:5] }
ResultList: {[a:2], [b:7], [c:5]}

User definition:

public class User { 
    private String name;
    private int comments;
}

My method:

public List<User> addTwoList(List<User> first, List<User> sec) {
    List<User> result = new ArrayList<>();
    for (int i=0; i<first.size(); i++) {
        Boolean bsin = false;
        Boolean isin = false;
        for (int j=0; j<sec.size(); j++) {
            isin = false; 
            if (first.get(i).getName().equals(sec.get(j).getName())) {
                int value= first.get(i).getComments() + sec.get(j).getComments();
                result.add(new User(first.get(i).getName(), value));
                isin = true;
                bsin = true;
            }
            if (!isin) {result.add(sec.get(j));}
        }
        if (!bsin) {result.add(first.get(i));}
    }
    return result;      
}

But it adds a whole lot of things to the list.

Upvotes: 5

Views: 138

Answers (4)

davidxxx
davidxxx

Reputation: 131346

As alternative fairly straight and efficient :

  • stream the elements
  • collect them into a Map<String, Integer> to associate each name to the sum of comments (int)
  • stream the entries of the collected map to create the List of User.

Alternatively for the third step you could apply a finishing transformation to the Map collector with collectingAndThen(groupingBy()..., m -> ... but I don't find it always very readable and here we could do without.

It would give :

List<User> users =
        Stream.concat(first.stream(), second.stream())
              .collect(groupingBy(User::getName, summingInt(User::getComments)))
              .entrySet()
              .stream()
              .map(e -> new User(e.getKey(), e.getValue()))
              .collect(toList());

Upvotes: 2

Nikolas
Nikolas

Reputation: 44398

There is a pretty direct way using Collectors.groupingBy and Collectors.reducing which doesnt require setters, which is the biggest advantage since you can keep the User immutable:

Collection<Optional<User>> d = Stream
    .of(first, second)                    // start with Stream<List<User>> 
    .flatMap(List::stream)                // flatting to the Stream<User>
    .collect(Collectors.groupingBy(       // Collecting to Map<String, List<User>>
        User::getName,                    // by name (the key)
                                          // and reducing the list into a single User

        Collectors.reducing((l, r) -> new User(l.getName(), l.getComments() + r.getComments()))))
    .values();                            // return values from Map<String, List<User>>

Unfortunately, the result is Collection<Optional<User>> since the reducing pipeline returns Optional since the result might not be present after all. You can stream the values and use the map() to get rid of the Optional or use Collectors.collectAndThen*:

Collection<User> d = Stream
    .of(first, second)                    // start with Stream<List<User>> 
    .flatMap(List::stream)                // flatting to the Stream<User>
    .collect(Collectors.groupingBy(       // Collecting to Map<String, List<User>>       
        User::getName,                    // by name (the key)
        Collectors.collectingAndThen(     // reduce the list into a single User

            Collectors.reducing((l, r) -> new User(l.getName(), l.getComments() + r.getComments())), 
            Optional::get)))              // and extract from the Optional
    .values();

* Thanks to @Aomine

Upvotes: 2

fps
fps

Reputation: 34460

You have to use an intermediate map to merge users from both lists by summing their ages.

One way is with streams, as shown in Aomine's answer. Here's another way, without streams:

Map<String, Integer> map = new LinkedHashMap<>();
list1.forEach(u -> map.merge(u.getName(), u.getComments(), Integer::sum));
list2.forEach(u -> map.merge(u.getName(), u.getComments(), Integer::sum));

Now, you can create a list of users, as follows:

List<User> result = new ArrayList<>();
map.forEach((name, comments) -> result.add(new User(name, comments)));

This assumes User has a constructor that accepts name and comments.

EDIT: As suggested by @davidxxx, we could improve the code by factoring out the first part:

BiConsumer<List<User>, Map<String, Integer>> action = (list, map) -> 
        list.forEach(u -> map.merge(u.getName(), u.getComments(), Integer::sum));

Map<String, Integer> map = new LinkedHashMap<>();
action.accept(list1, map);
action.accept(list2, map);

This refactor would avoid DRY.

Upvotes: 3

Ousmane D.
Ousmane D.

Reputation: 56423

This is better done via the toMap collector:

 Collection<User> result = Stream
    .concat(first.stream(), second.stream())
    .collect(Collectors.toMap(
        User::getName,
        u -> new User(u.getName(), u.getComments()),
        (l, r) -> {
            l.setComments(l.getComments() + r.getComments());
            return l;
        }))
    .values();
  • First, concatenate both the lists into a single Stream<User> via Stream.concat.
  • Second, we use the toMap collector to merge users that happen to have the same Name and get back a result of Collection<User>.

if you strictly want a List<User> then pass the result into the ArrayList constructor i.e. List<User> resultSet = new ArrayList<>(result);

Kudos to @davidxxx, you could collect to a list directly from the pipeline and avoid an intermediate variable creation with:

List<User> result = Stream
    .concat(first.stream(), second.stream())
    .collect(Collectors.toMap(
         User::getName,
         u -> new User(u.getName(), u.getComments()),
         (l, r) -> {
              l.setComments(l.getComments() + r.getComments());
              return l;
         }))
    .values()
    .stream()
    .collect(Collectors.toList());

Upvotes: 5

Related Questions