3
\$\begingroup\$

I am using a function in a SpringBoot @RestController, with Lombok

This function is an intermediate result that permit me to do an aggregation between two classes.

This 2 classes are:

@Data
public class AchMatiereCmdCptAggDto implements Serializable {

    private static final long serialVersionUID = 4022154955808447185L;
    private String codService;
    private String codSociete;
    private String numCompte;
    private String codCommande;
    private Double mntCommandeHTLCY;
    private Double mntLivreHTLCY;
    private Double mntFactureHTLCY;
}

@Getter
@Setter
public class AchMatiereCompteCommandeAggFullDto extends AchMatiereCmdCptAggDto implements Serializable {

    private static final long serialVersionUID = 6357406952790844723L;
    private Date dateCommande;
    private String codFournisseur;
    private String libFournisseur;
    private Boolean booFournisseurIntragroupe;
    private Long idCommandeEtat;

}

public static String stringConcat(String ... elements) {
    return new StringBuilder().append(elements).toString();
}

I use them that way, and I want to improve forEach part, and any other improvable piece of code:

 public List<AchMatiereCompteCommandeAggFullDto> mergeAchat(List<AchMatiereCompteCommandeAggFullDto> achats,


List<AchMatiereCmdCptAggDto> achats2) {

    Set<AchMatiereCmdCptAggDto> setAchats = new HashSet<>(achats2);
    Map<String, AchMatiereCmdCptAggDto> map = setAchats.stream()
            .collect(Collectors.toMap(
                    p -> stringConcat(p.getNumCompte(), p.getCodCommande()),
                    p -> p, (ach1, ach2) -> ach2
            ));

    achats.stream().forEach(achat -> {
        String k = stringConcat(achat.getCodSociete(), achat.getCodService(), achat.getNumCompte(), achat.getCodCommande());

        if (map.containsKey(k)) {
            achat.setMntFactureHTLCY(map.get(k).getMntFactureHTLCY());
        }
    });
    return achats;
}

I was thinking about using a StringBuilder for the concatenation of attributes.

I would also like to avoid forEach, and use a Map or something like this.

Thanks for your help.

\$\endgroup\$
2
\$\begingroup\$

From java 8 it is available in the String class the method join that achieves the same result of your method stringConcat so instead of:

String k = stringConcat(achat.getCodSociete(), achat.getCodService(), achat.getNumCompte(), achat.getCodCommande());

You can use String.join with "" delimiter:

String k = String.join("", achat.getCodSociete(), achat.getCodService(), achat.getNumCompte(), achat.getCodCommande());

It is also possibile to avoid forEach and use Map instead but because you are modifying the achats list elements, you have to map every element of the list to a new instance of your class and after you obtain your new list. To achieve this you can define a new copy constructor for the class:

achats.stream()
    .map(original -> {
        String k = //omitted for brevity : see above the code for k
        AchMatiereCompteCommandeAggFullDto copy = new AchMatiereCompteCommandeAggFullDto(original);
        if (map.containsKey(k)) {
            copy.setMntFactureHTLCY(map.get(k).getMntFactureHTLCY());
        }
        return copy;
    }).collect(Collectors.toList()); //<- return the new list 
\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, is String.join better than StringBuiler ? in terms of performance for example ? On the other hand, in terms of performance, do you think that its better without forEach ? \$\endgroup\$ – Driss Nejjar Sep 11 at 6:55
  • \$\begingroup\$ @DrissNejjar, in this case if you would to use StringBuilder , you should iterate over fields of you class , appending them in your stringbuilder and after convert it to String, instead with String.join you have already a method to do it. In terms of performance your custom method maybe is better, but I don't think you can notice it in a complicate code. For the second question, the use of map implies the creation of a new list of modified old elements (all new instances) and I prefer your old solution forEach. Probably the use of stream is an overkill, better loop over list. \$\endgroup\$ – dariosicily Sep 11 at 8:44
  • \$\begingroup\$ Okey for StringBuilder and String.join. if you think that stream is an overkill, what do you suggest instead ? when you say loop over list, do you think about a for loop ? Thanks \$\endgroup\$ – Driss Nejjar Sep 11 at 8:53
  • \$\begingroup\$ @DrissNejjar Yes, just a for loop of list and inside call the setter method if condition is true, so no use of Stream in this case. \$\endgroup\$ – dariosicily Sep 11 at 9:04
  • 1
    \$\begingroup\$ If you want to use Map solution you have to define a copy constructor or equivalent method to create new instances of your class and after obtain the new list; otherwise if you don't want to touch code of your class use your forEach solution you already have. \$\endgroup\$ – dariosicily Sep 11 at 14:20

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.