Can I replace this For loop using a ForEach

Question

The following code works fine, but out of curiosity I was wondering if I can replace the For loop with a ForEach

private static List<SubjectRole> getSubjectRoles(List<ca.payment.simulator.model.api.SubjectRole> subjectRoles) {
    List<SubjectRole> subjectRoleList = new ArrayList<>();

    for (ca.payment.simulator.model.api.SubjectRole subjectRole : subjectRoles) {
        SubjectRole subjectRoleStream = new SubjectRole();
        subjectRoleStream.setSubjectRole(subjectRole.getSubjectRole());
        subjectRoleList.add(subjectRoleStream);
    }

    return subjectRoleList;
}

changing to foreach doesn't make much sense. You should use stream, map and collect for full use of stream API — Turo
When you wonder whether something will work, what is it that stops you from just trying it? — MarsAtomic
I'm confused, why are you making a new SubjectRole, and then doing setSubjectRole (???) with the values from iteration? Are you just trying to make a copy list? — Rogue

Allen D. Ball Allen D. Ball · Accepted Answer · 2020-08-04T22:06:54

You can but it is not likely to make the code more readable. With Stream.forEach():

        List<SubjectRole> subjectRoleList = new ArrayList<>();

        subjectRoles.stream()
            .forEach(t -> {
                    SubjectRole subjectRoleStream = new SubjectRole();
                    subjectRoleStream.setSubjectRole(t.getSubjectRole());
                    subjectRoleList.add(subjectRoleStream);
                });
        }

        return subjectRoleList;

You could use .map()/.collect() to let the Streams API manage the List allocation:

        List<SubjectRole> subjectRoleList =
            subjectRoles.stream()
            .map(t -> {
                    SubjectRole subjectRoleStream = new SubjectRole();
                    subjectRoleStream.setSubjectRole(t.getSubjectRole());
                    return subjectRoleStream;
                })
            .collect(Collectors.toList());

        return subjectRoleList;

To get more concise, you need to somehow combine the instantiation and attribute set to one call (method or constructor). If there isn't an existing method, you could create a private helper:

    private static List<SubjectRole> getSubjectRoles(List<ca.payment.simulator.model.api.SubjectRole> subjectRoles) {
        List<SubjectRole> subjectRoleList =
            subjectRoles.stream()
            .map(t -> copy(t))
            .collect(Collectors.toList());

        return subjectRoleList;
    }

    private static SubjectRole copy(SubjectRole in) {
        SubjectRole out = new SubjectRole();

        out.setSubjectRole(in.getSubjectRole());

        return out;
    }

Can I replace this For loop using a ForEach

1 Answers