patternjavaMinor
RXJava operate on exiting List instead of creating new
Viewed 0 times
newrxjavacreatingoperateinsteadlistexiting
Problem
I'm dipping my toes in to RXJava for the first time. The function below works just fine but I can't help thinking it can be improved. For instance I'm sure that I'm unnecessarily creating a new
List sortedDishes when I could be performing operations on the exiting List dishes that is passed in, I just don't know how yet.public void sortDishes(int categoryId, List dishes) {
final List sortedDishes = new ArrayList<>();
final Integer[] subCategories = dbInteractor.getSubCategories(categoryId);
final ArrayList cats = new ArrayList<>();
cats.add(categoryId);
if (subCategories != null) {
cats.addAll(Arrays.asList(subCategories));
}
Observable.from(dishes)
.filter(new Func1() {
@Override
public Boolean call(Dish dish) {
Integer[] categories = categoryHelper.getCategorys(dish.getCategories());
for (Integer category : categories) {
if (cats.contains(category)) {
return true;
}
}
return false;
}
})
.subscribe(new Subscriber() {
@Override
public void onCompleted() {
}
@Override
public void onError(Throwable e) {
}
@Override
public void onNext(Dish dish) {
sortedDishes.add(dish);
}
});
}Solution
You're just abusing the library here. What you're doing can be done significantly more succinctly in standard java API:
Note that this corrects a typo in
Further notes
Your dbInteractor's API could use a rework. I generally advocate against returning
I strongly suggest you consider returning
I also don't understand why the method is typed
public void sortDishes(int categoryId, List dishes) {
final Set categories = new HashSet();
categories.add(categoryId);
final Integer[] subCategories = dbInteractor.getSubCategories(categoryId);
if (subCategories != null) {
categories.addAll(Arrays.asList(subCategories));
}
final List sortedDishes = dishes.stream()
.filter(d -> categoryHelper.getCategories(d.getCategories())
.stream().anyMatch(categories::contains))
.sort()
.collect(Collectors.toList());
}Note that this corrects a typo in
getCategorys.Further notes
Your dbInteractor's API could use a rework. I generally advocate against returning
null from APIs that return a collection of elements, whether that's an Array or an actual Collection subclass.I strongly suggest you consider returning
new Integer[0] or even Collections.emptyList() because this will greatly simplify calling code by guaranteeing you're not returning null.I also don't understand why the method is typed
void. Either you actually modify the List you've received or you return a new List.Code Snippets
public void sortDishes(int categoryId, List<Dish> dishes) {
final Set<Integer> categories = new HashSet<Integer>();
categories.add(categoryId);
final Integer[] subCategories = dbInteractor.getSubCategories(categoryId);
if (subCategories != null) {
categories.addAll(Arrays.asList(subCategories));
}
final List<Dish> sortedDishes = dishes.stream()
.filter(d -> categoryHelper.getCategories(d.getCategories())
.stream().anyMatch(categories::contains))
.sort()
.collect(Collectors.toList());
}Context
StackExchange Code Review Q#126055, answer score: 2
Revisions (0)
No revisions yet.