0
\$\begingroup\$

Lets say I have Cars with Features, and I have been given a List of features to look for in the Cars available with me. Though I could have done that using for-loop, what are the suggestions for below? Is this production-worthy?

import java.util.Arrays;
import java.util.List;

import org.apache.commons.collections4.CollectionUtils;

public class LookupTest {

    public static void main(String[] args) {
        Feature ft11 = new Feature(1);
        Feature ft12 = new Feature(2);
        Feature ft13 = new Feature(4);
        Car fer = new Car("Ferrari", ft11, ft12, ft13);

        Feature ft21 = new Feature(1);
        Feature ft22 = new Feature(2);
        Feature ft23 = new Feature(8);
        Car suz = new Car("Suzuki", ft21, ft22, ft23);

        List<Car> cars = Arrays.asList(fer, suz);
        List<Integer> lookForTypes = Arrays.asList(4); // look whether any car has feature-type 4

        if (CollectionUtils.isNotEmpty(lookForTypes)) {
            /* code block in question : start */
            boolean atLeastOneTypeFound = cars
                    .parallelStream()
                    .anyMatch(holding -> holding.getAmounts()
                            .parallelStream()
                            .anyMatch(feature -> {
                                return lookForTypes.contains(feature.type);
                            }));
            /* code block in question : end */    
            System.out.println("atLeastOneTypeFound=" + atLeastOneTypeFound);
        }
    }
}

class Car {
    List<Feature> featureList;
    String name;

    public Car(String name, Feature... features) {
        featureList = Arrays.asList(features);
        this.name = name;
    }

    public List<Feature> getAmounts() {
        return featureList;
    }

    public String getName() {
        return name;
    }
}

class Feature {
    int type;

    public Feature(int type) {
        this.type = type;
    }

    public int getType() {
        return type;
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ I have rolled back your last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Heslacher
    Commented Jan 8, 2019 at 15:00
  • \$\begingroup\$ Ok, that was a typo actually. \$\endgroup\$
    – xploreraj
    Commented Jan 8, 2019 at 19:49

1 Answer 1

1
\$\begingroup\$

If you don't need the Car, you can also flatMap to have the features and then distinct them before searching.

cars.stream()
  .flatMap(car -> car.getAmounts().stream())
  .map(Feature::getType)
  .distinct() // As spotted by @Pimgd in the comments this is useless
  .anyMatch(lookForTypes::contains);

But if you want to improve the readability of your code you should better consider to move a part of this "logic" into your objects :

cars.stream()
  .anyMatch(car -> car.hasAtLeastOneFeature(lookForTypes);

// ...

Car {
  boolean hasAtLeastOneFeature(List<Integer> features) {
    // Note that I renamed getAmounts to getFeatures
    return getFeatures().stream()
      .anyMatch(feature -> features.contains(feature.getType());
  }
}
\$\endgroup\$
5
  • 2
    \$\begingroup\$ Why distinct before searching? anyMatch would stop upon finding a hit, so what's the point in making a list of uniques first? \$\endgroup\$
    – Pimgd
    Commented Jan 8, 2019 at 14:42
  • \$\begingroup\$ What is the difference between doing .parallelStream().anyMatch() and your flatMap(stream) -> map() -> anyMatch() implementation? I dont need the Car. \$\endgroup\$
    – xploreraj
    Commented Jan 9, 2019 at 3:14
  • \$\begingroup\$ @Pimgd indeed, there is no gain to distinct first. @xploreraj using flatMap you avoid the nested Stream<Stream> so that your code is a bit more readable. \$\endgroup\$
    – gervais.b
    Commented Jan 9, 2019 at 6:55
  • \$\begingroup\$ @gervais.b But is it not like stream as opposed to parallelStream ? \$\endgroup\$
    – xploreraj
    Commented Jan 9, 2019 at 19:49
  • \$\begingroup\$ You can use flatMap on with stream and parallelStream without changes. I used stream in my sample because the word is shorter. \$\endgroup\$
    – gervais.b
    Commented Jan 10, 2019 at 8:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.