I keep getting told it is bad practice to not terminate a Stream via methods such as collect and findFirst but no real feedback as to why not much said about it in blogs.
Looking at following example, instead of using a massive nested if check, I went with Optional to get back a List value. As you can see my last step is filter in that Stream. This works as expected for me which is to get back a list. Why is this wrong and how should I have written it instead?
import lombok.Getter;
import lombok.Setter;
import java.util.*;
public class Main {
public static void main(String[] args) {
RequestBean requestBean = new RequestBean();
// if I uncomment this I will get the list values printed as expected
// FruitBean fruitBean = new FruitBean();
// AnotherBean anotherBean = new AnotherBean();
// InnerBean innerBean = new InnerBean();
// requestBean.setFruitBeans(Collections.singletonList(fruitBean));
// fruitBean.setAnotherBeans(Collections.singletonList(anotherBean));
// anotherBean.setInnerBeans(Collections.singletonList(innerBean));
// List<String> beans = Arrays.asList("apple", "orange");
// innerBean.setBeans(beans);
List<String> result = getBeanViaOptional(requestBean);
if(result != null){
for(String s : result){
System.out.println(s);
}
}else {
System.out.println("nothing in list");
}
}
private static List<String> getBeanViaOptional(RequestBean bean){
Optional<List<String>> output = Optional.ofNullable(bean)
.map(RequestBean::getFruitBeans)
.map(n -> n.get(0))
.map(FruitBean::getAnotherBeans)
.map(n -> n.get(0))
.map(AnotherBean::getInnerBeans)
.map(n -> n.get(0))
.map(InnerBean::getBeans)
// why is this bad practice to end with a filter. how should I write this then?
.filter(n -> n.contains("apple"));
if(!output.isPresent()){
throw new CustomException();
}
return output.get();
}
// not using this. just to show that optional was preferable compared to this.
private static List<String> getBeanViaIfChecks(RequestBean bean){
if(bean != null){
if(bean.getFruitBeans() != null){
if(bean.getFruitBeans().get(0) != null){
if(bean.getFruitBeans().get(0).getAnotherBeans() != null){
if(bean.getFruitBeans().get(0).getAnotherBeans().get(0) != null){
if(bean.getFruitBeans().get(0).getAnotherBeans().get(0).getInnerBeans() != null){
if(bean.getFruitBeans().get(0).getAnotherBeans().get(0).getInnerBeans().get(0) != null){
return bean.getFruitBeans().get(0).getAnotherBeans().get(0).getInnerBeans().get(0).getBeans();
}
}
}
}
}
}
}
return null;
}
}
@Getter
@Setter
class RequestBean{
List<FruitBean> fruitBeans;
}
@Getter
@Setter
class FruitBean{
List<AnotherBean> anotherBeans;
}
@Getter
@Setter
class AnotherBean{
List<InnerBean> innerBeans;
}
@Getter
@Setter
class InnerBean{
List<String> beans;
}
class CustomException extends RuntimeException{
// do some custom exception stuff
}
filter
method comes fromOptional
and notStream
.get(0)
occurs and while I am not certain of a cleaner way to avoid them. They do itch my eyes while reading a functional approach.for
loop approach is not equivalent to the functional approach since it doesn't ever throw aCustomException
.