Reputation: 609
I would like to split an ArrayList
that I am looping trough and set a field called active
which can be true
or false
. But at the end of loop I would like to split this collection in two groups.. active = false
and active = true
, so doing this I won't need to search in database twice..
for example:
private List<Classes> searchClasses(ClassItems listItems) {
List<ClassItem> items = new ArrayList<ClassItem>();
for (Iterator<ClassItem> iterator = listItems.getItems().iterator(); iterator.hasNext();) {
ClassItems item = iterator.next();
ClassEntityManager classEnt = ClassEntityManager.search(item.getId);
if(classEnt.active()){
item.setActive(true);
items.add(item);
}
}
return items;
}
What is the best approach to do this??
Upvotes: 3
Views: 82
Reputation: 1454
Use two collections, one for actives and the other for not actives.
When you fetch the data from the DB, simply put the CalssItem
in the correct list:
private List<ClassItem> searchClasses(ClassItems listItems) {
List<ClassItem> activeItems= new ArrayList<ClassItem>();
List<ClassItem> notActiveItems= new ArrayList<ClassItem>();
Iterator<ClassItem> i = listItems.getItems().iterator();
while(i.hasNext()) { //This is a better approach.
ClassEntityManager classEnt = ClassEntityManager.search(i.next().getId);
if(classEnt.active()){
item.setActive(true);
activeItems.add(item);
}else{
item.setActive(false);
notActiveItems.add(item);
}
}
List<ClassItem> ret = new ArrayList<ClassItem>(activeItems);
ret.addAll(notActiveItems);
return ret;
}
BUT, in this way, both activeItems
and notActiveItems
are unreacheable. Best thing to do is to have a loop outside your method that checks if the ClassItem
is active or not. In this way both activeItems
and notActiveItems
can be deleted from the method:
private List<ClassItem> searchClasses(ClassItems listItems) {
List<ClassItem> items= new ArrayList<ClassItem>();
Iterator<ClassItem> i = listItems.getItems().iterator();
while(i.hasNext()) { //This is a better approach.
ClassEntityManager classEnt = ClassEntityManager.search(i.next().getId);
item.setActive(classEnt.active());
items.add(item);
}
return items;
}
And to use the list:
List<ClassItem> items = searchClasses(classItems);
for(ClassItem item: items){
if(item.isActive()){
//do something
}else{
//do something else
}
}
Better yet is to use the magnificient and beautiful Java 8 Stream API:
List<ClassItem> active = items.stream().filter(x->x.isActive).collect(Collectors.toList());
List<ClassItem> notActive = items.stream().filter(x->!x.isActive).collect(Collectors.toList());
or the one liner:
List<ClassItem> active = searchClasses(classItems).stream().filter(x->x.isActive).collect(Collectors.toList());
NOTES:
Your code has a return type of List<Classes>
, while the returned value is of List<ClassItem>
. Which is right?
Your iterator has a generic type of ClassItem
while the next()
method returns a ClassItems
object. Which is right?
Upvotes: 0
Reputation: 40500
Make two lists instead of one.
if(classEnt.active()) {
activeItems.add(item);
item.setActive(true);
} else {
inactiveItems.add(item);
}
Upvotes: 2