5
votes

Ajout de deux listes de type propre

J'ai une classe User simple avec une propriété String et une propriété int .

Je voudrais ajouter deux listes d'utilisateurs de cette façon:

  • si la chaîne est égale, les nombres doivent être ajoutés et ce serait sa nouvelle valeur.
  • La nouvelle liste doit inclure tous les utilisateurs avec des valeurs appropriées.

Comme ceci:

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;      
}

Définition de l'utilisateur :

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

Ma méthode:

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

Mais cela ajoute beaucoup de choses à la liste.


0 commentaires

4 Réponses :


5
votes

Ceci est mieux fait via le collecteur toMap :

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());
  • Tout d'abord, concaténez les deux listes en un seul Stream via Stream.concat .
  • Deuxièmement, nous utilisons le collecteur toMap pour fusionner les utilisateurs qui ont le même nom et récupérer un résultat de Collection code >.

si vous voulez strictement une List alors passez le résultat dans le constructeur ArrayList c'est-à-dire List resultSet = new ArrayList (result);


Félicitations à @davidxxx, vous pourriez collecter dans une liste directement à partir du pipeline et éviter une création de variable intermédiaire avec:

 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();


10 commentaires

Ou pourquoi pas .values ​​(). Stream (). Collect (toList ()) pour éviter de définir une variable supplémentaire.


@davidxxx si l'on ne veut pas la création d'une variable, alors je suggérerais de passer la majeure partie du pipeline directement au constructeur, ce qui conduit à une moindre lisibilité, donc oui, votre suggestion est un bon point en effet. Je vais modifier pour accommoder.


En effet, IntelliJ le propose. C'est fou :) Votre façon de collecter est simple mais l'idiome pour renvoyer l'objet fusionné est moche (ce n'est pas votre faute bien sûr) car Java ne repose pas beaucoup sur l'implicite. Une approche immuable pourrait rendre les choses plus lisibles tout en ayant des conséquences sur le temps d'exécution: toMap (User :: getName, u -> new User (u.getName (), u.getComments ()), (l, r) -> nouvel utilisateur (l.getName (), l.getComments () + r.getComments ()))


@davidxxx En effet, c'est une autre option, mais je resterais à l'écart car vous avez déjà laissé entendre que ce n'est pas convivial pour la mémoire. une meilleure option serait d'extraire la fonction de fusion dans une méthode à l'intérieur de User , puis de l'appeler avec une référence de méthode du pipeline, ce qui conduirait à une meilleure lisibilité et éviterait la surcharge. exemple la méthode ressemblerait à public User merge (User other) {setComments (getComments () + other.getComments ()); renvoyez ceci; } puis utilisez User :: merge à la place de la fonction de fusion ci-dessus.


@Aomine: Merci et vous êtes les bienvenus. Le plus gros problème avec java-stream est la lisibilité et la capacité à déterminer ce dont un collecteur a exactement besoin.


@Aomnine Je comprends votre POV mais notez que l'idiome que vous utilisez pour créer un nouvel objet pour chaque élément de flux n'est pas non plus compatible avec la mémoire / processeur: -> new User (u.getName (), u.getComments ()) Supposons que vous ayez 10 éléments avec la même clé en moyenne, cela signifie que vous créez 9 fois plus d'objets que nécessaire.


@davidxxx auquel cas, si l'utilisation de la mémoire est vraiment préoccupante, j'envisagerais d'utiliser .collect (groupingBy (User :: getName, summingInt (User :: getComments))) , puis mapper chaque entrée à un objet User . ;-)


Ça sonne bien. Et le groupingBy () est de cette manière beaucoup plus explicite. Je publierais quelque chose de très proche hier mais je n'avais pas le temps pour.


@davidxxx ouais, ce serait bien si vous le postiez. @ moi quand tu le fais.


Au-delà de l'utilisation de la mémoire ou du processeur, je trouve cela vraiment plus soigné. La fonction de fusion de toMap () diminue la lisibilité de la même manière que collectThen () . Personnellement, j'essaye de les éviter autant que possible. J'ai posté la réponse alors que vous aviez déjà donné l'essentiel dans votre commentaire :)



3
votes

Vous devez utiliser une carte intermédiaire pour fusionner les utilisateurs des deux listes en additionnant leurs âges.

Une façon est d'utiliser les flux, comme indiqué dans Réponse d'Aomine . Voici une autre manière, sans flux:

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);

Vous pouvez maintenant créer une liste d'utilisateurs, comme suit:

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

Cela suppose que User a un constructeur qui accepte name et comments.


EDIT: strong> Comme suggéré par @davidxxx, nous pourrions améliorer le code en prenant en compte la première partie:

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));

Ce refactor éviterait DRY.


10 commentaires

Bien :) Vous pouvez factoriser la première partie: Stream.concat (list1.stream (), list2.stream ()). ForEach (u -> ...)


@davidxxx Je pensais exactement la même chose;) Mais toucher une carte depuis Stream.forEach n'est pas compatible avec le flux (c'est déconseillé dans la documentation) ...


@FedericoPeraltaSchaffner: Les documents découragent de Stream :: forEach qui est très différent de List :: forEach (ou Iterable :: forEach pour être exact ) qui se comporte exactement comme la boucle for traditionnelle. Qu'est-ce que tu penses? Je ne suis pas sûr.


@Nikolas ce que Federico voulait dire, c'est que l'utilisation de Stream.concat (list1.stream (), list2.stream ()). ForEach (u -> ...) comme suggéré par david entraînerait des effets secondaires inutiles comme vous vous accumulez ensuite sur une carte en externe. les flux et les effets secondaires ne doivent pas aller de pair. c'est encore plus vrai lors de l'exécution en parallèle. Federico corrige-moi si je me trompe ;-) mais je pense que c'est ce que tu voulais dire.


@Aomine: je parle de (liste, carte) -> list.forEach . Le stream n'est pas du tout construit et la méthode forEach provoque des effets secondaires - ce qui est bien. Pourquoi le Stream :: forEach est déconseillé car il est identique à Iterable :: forEach ? Je comprends parfaitement le problème des effets secondaires avec les flux parallèles, je ne recommande pas d'utiliser ex. stream-map-filter-forEach , mais Stream :: forEach pourrait être bien ?.


@Aomine je voulais dire exactement ça :)


@Nikolas Oui, je pense que vous avez raison. Il n'y aurait aucun problème dans ce cas précis. Cependant, je préfère prendre cela strictement et ne jamais utiliser ce modèle. Ce serait juste pour des raisons stylistiques , si vous préférez ... D'ailleurs, même si cela n'arrivera jamais, vous ne savez pas si Stream.concat rend le flux parallèle ou pas (ça ne devrait pas, mais) ...


C'est ce que je voulais dire, merci. Le comportement Stream.concat est nouveau pour moi, il devient parallèle si l'une des entrées est parallèle (d'après la documentation). Cela implique-t-il que l'utilisation de Stream.of (...). FlatMap (List :: stream) ... est plus sûre dans ce cas?


@Nikolas Je ne pense pas ... C'est aussi sûr que d'utiliser Stream.concat (qui reçoit 2 flux, et vous pouvez contrôler si les deux sont parallèles ou non), bien qu'il ait le avantage de vous permettre de concater plus de 2 collections.


J'aime beaucoup la mise à jour. Très soigné tout en étant un peu bavard.



2
votes

Il existe une manière assez directe d'utiliser Collectors.groupingBy et Collectors.reducing qui ne nécessite pas de setters, ce qui est le plus grand avantage puisque vous pouvez garder User immutable final:

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();  

Malheureusement , le résultat est Collection> car le pipeline de réduction renvoie Optional car le résultat peut ne pas être présent après tout. Vous pouvez diffuser les valeurs et utiliser map () pour vous débarrasser de Optional ou utiliser Collectors.collectAndThen *:

XXX

* Merci à @ Aomine


2 commentaires

vous pouvez passer le collecteur reduction au collecteur collectionAndThen avec Optional :: get comme fonction de finition pour rendre les choses plus courtes et plus faciles, par exemple .collect (Collectors.groupingBy (User :: getName, CollectAndThen (Collectors.reducing ((l, r) -> new User (l.getName (), l.getComments () + r.getComments ())) , Facultatif :: get))) . cela signifie que vous n'avez pas besoin des .values ​​(). stream () ..... suivants


Génial! Le mieux pour répondre aux questions est le fait que j'en apprends plus. Je vous remercie :)



2
votes

Comme alternative assez simple et efficace:

  • diffuser les éléments
  • rassemblez-les dans une Map pour associer chaque nom à la somme des commentaires ( int )
  • diffusez les entrées de la carte collectée pour créer la liste des utilisateurs.

Alternativement, pour la troisième étape, vous pouvez appliquer une transformation de finition au collecteur Map avec collectionAndThen (groupingBy () ..., m -> ... code> mais je ne le trouve pas toujours très lisible et ici on pourrait s'en passer.

Cela donnerait:

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());


0 commentaires