Reputation: 418
I have written a code using Java 8 streams but trying to refactor.
I have filtered the list with different criteria and storing it into a new list. Is there any way to filter all the criteria at once and store the group of result into a list. I don't know how to explain that.
Here is my code and just need to refactor it.
List<A> finalList = new ArrayList<>();
List<A> aList= new ArrayList<>();
A objectA = new A();
List<B> bList = new ArrayList<>();
bList.add(new B(5700));
bList.add(new B(5750));
bList.add(new B(5800));
bList.add(new B(5812));
bList.add(new B(5803));
bList.add(new B(5802));
objectA.setMyList(bList);
aList.add(objectA);
aList.stream()
.forEach(
a -> {
if(!a.getMyList.isEmpty()){
List<B> groupA =
a.getMyList.stream()
.filter(b -> b.getType() == 5700 || b.getType() == 5750)
.collect(Collectors.toList());
List<B> groupB =
a.getMyList.stream()
.filter(b -> b.getType() == 5800 || b.getType() == 5810)
.collect(Collectors.toList());
List<B> groupC =
a.getMyList.stream()
.filter(b -> b.getType() == 5802 || b.getType() == 5812)
.collect(Collectors.toList());
List<B> groupD =
a.getMyList.stream()
.filter(b -> b.getType() == 5803 || b.getType() == 5813)
.collect(Collectors.toList());
if(!groupA.isEmpty()){
finalList.add(getAWithGroupList(a, groupA));
}
if(!groupB.isEmpty()){
finalList.add(getAWithGroupList(a, groupB));
}
if(!groupC.isEmpty()){
finalList.add(getAWithGroupList(a, groupC));
}
if(!groupD.isEmpty()){
finalList.add(getAWithGroupList(a, groupD));
}
}
}
);
private A getAWithGroupList(A a, List<B> group) {
A a = (A) a.clone();
a.setMyList(group);
return a;
}
Any help or suggestions would be appreciated.
Thanks.
Upvotes: 1
Views: 1012
Reputation: 298579
The biggest obstacle is that the rationale behind your criteria is unknown. So when we can’t simplify it, we have to declare these group explicitly:
Integer[][] groups={ { 5700, 5750 }, {5800, 5810}, {5802, 5812}, {5803, 5813} };
Map<Integer, Integer[]> type2group = Arrays.stream(groups)
.collect(HashMap::new, (m,g) -> Arrays.stream(g).forEach(i->m.put(i, g)), Map::putAll);
ArrayList<A> finalList = aList.stream()
.map(a -> a.getMyList().stream().collect(Collectors.collectingAndThen(
Collectors.groupingBy(b -> type2group.get(b.getType())),
group2b -> Arrays.stream(groups)
.map(g -> group2b.getOrDefault(g, Collections.emptyList()))
.map(listB -> getAWithGroupList(a, listB))
.collect(Collectors.toList()))))
.collect(ArrayList::new, List::addAll, List::addAll);
The two dimensional array is the easiest-to-maintain way to specify these groups. To process them efficiently afterward, the first step converts them into a map from type (result of B.getType
) to an identifier of the desired group, which is just a subarray of our complete group
specifier (the actual type of the group identifier is irrelevant as we will encounter the same objects later-on, so the fact that arrays have no equals
method doesn’t matter here).
The second step performs the grouping of the B
instances for each A
and assembles the finalList
.
Note that if there are absent groups, the above solution will create empty lists for them. If you don’t want to have them, you can change the last step of the solution to
ArrayList<A> finalList = aList.stream()
.map(a -> a.getMyList().stream().collect(Collectors.collectingAndThen(
Collectors.groupingBy(b -> type2group.get(b.getType())),
group2b -> Arrays.stream(groups)
.map(g -> group2b.get(g)).filter(Objects::nonNull)
.map(listB -> getAWithGroupList(a, listB))
.collect(Collectors.toList()))))
.collect(ArrayList::new, List::addAll, List::addAll);
Alternatively, the following code is more compact, but less efficient as it will process the data multiple times (traverse all B
s for each group and perform linear search within the groups):
Integer[][] groups={ { 5700, 5750 }, {5800, 5810}, {5802, 5812}, {5803, 5813} };
List<A> finalList = aList.stream().flatMap(a -> Arrays.stream(groups)
.map(g -> getAWithGroupList(a, a.getMyList().stream()
.filter(b -> Arrays.asList(g).contains(b.getType()))
.collect(Collectors.toList()))))
.collect(Collectors.toList());
In case there might be absent groups and you don’t want to have empty lists for them, you can change it to
List<A> finalList = aList.stream().flatMap(a -> Arrays.stream(groups)
.map(g -> a.getMyList().stream()
.filter(b -> Arrays.asList(g).contains(b.getType()))
.collect(Collectors.toList()))
.filter(l -> !l.isEmpty())
.map(l -> getAWithGroupList(a, l)))
.collect(Collectors.toList());
Upvotes: 1
Reputation: 1544
Simple. Wrap the repeating code in a method:
private List<B> listifyAndfilter(A a, Predicate<List<B>> check){
return a.getMyList.stream().filter(check).collect(Collectors.toList());
}
And then you can do:
List<B> groupA = listifyAndFilter(a, b -> b.getType() == 5700 || b.getType() == 5750),
groupB = listifyAndFilter(a, b -> b.getType() == 5800 || b.getType() == 5810),
groupC = listifyAndFilter(a, b -> b.getType() == 5802 || b.getType() == 5812),
groupD = listifyAndFilter(a, b -> b.getType() == 5803 || b.getType() == 5813);
Note the commas at the ends of the lines.
Unrelated but useful: You can also combine these if-statements:
if(!groupA.isEmpty()){
finalList.add(getAWithGroupList(a, groupA));
}
if(!groupB.isEmpty()){
finalList.add(getAWithGroupList(a, groupB));
}
if(!groupC.isEmpty()){
finalList.add(getAWithGroupList(a, groupC));
}
if(!groupD.isEmpty()){
finalList.add(getAWithGroupList(a, groupD));
}
into this for-each loop:
for (List<B> group : new List<B>[]{groupA, groupB, groupC, groupD}) // Packs the lists in an array and iterates over the array
if(!group.isEmpty())
finalList.add(getAWithGroupList(a, group));
Upvotes: 1