Java Stream over a list and check if the list contains at list one object with one of three given fi-CodePudding

Given a class Ball (simplified for this question), where I can not change the equals and hashCode method

class Ball {
    String color;
    //some more fields, getters, setters, equals, hashcode ..
}

and a list of balls, I want to return true if the list contains at least one ball for each color value "RED", "YELLOW" and "GREEN". Example inputs:

List<Ball> first = List.of(
        new Ball("RED"),
        new Ball("BLUE"),
        new Ball("GREEN"),
        new Ball("RED"),
        new Ball("YELLOW"),
        new Ball("RED"));

List<Ball> second = List.of(
        new Ball("RED"),
        new Ball("BLUE"),
        new Ball("GREEN"),
        new Ball("RED"));

expected result for first list is true and for second false. For now I have a classic loop and three counter variables:

private static boolean isValidList(final List<Ball> balls) {
    int r = 0;
    int y = 0;
    int g = 0;
    for (Ball ball : balls) {
        String color = ball.getColor();
        if("RED".equals(color)){
            r  ;
        }
        else if("YELLOW".equals(color)){
            y  ;
        }
        else if("GREEN".equals(color)){
            g  ;
        }

        if(r > 0 && y > 0 && g > 0){
            break;
        }
    }
    return r > 0 && y > 0 && g > 0;
}

I have tried to refactor it to use streams like below

private static boolean isValidListStreams(final List<Ball> balls) {
    long r = balls.stream().filter(ball -> "RED".equals(ball.getColor())).count();
    long y = balls.stream().filter(ball -> "YELLOW".equals(ball.getColor())).count();
    long g = balls.stream().filter(ball -> "GREEN".equals(ball.getColor())).count();
    return r > 0 && y > 0 && g > 0;
}

but the above need to stream over the list 3 times. Is there a way I can do it in one go? I can't do it with filter using or

return balls.stream()
            .filter(ball -> ball.getColor().equals("RED") ||
                            ball.getColor().equals("YELLOW") || 
                            ball.getColor().equals("GREEN")).count() >= 3;

since there may be multiple of the same color.

CodePudding user response：

I can't do it with filter using or since there may be multiple of the same color.

You can just use distinct to remove the duplicate colours.

Since you cannot modify equals, you should first map everything to their color first, then distinct and filter.

return balls.stream()
            .map(Ball::getColor)
            .distinct()
            .filter(color -> color.equals("RED") ||
                             color.equals("YELLOW") || 
                             color.equals("GREEN")).count() == 3;

Notice that your original for loop is short-circuiting - once you have found the three required colours, you stop looping. However, count will count everything. If that is undesirable, you can do a limit(3) before it.

Also, replacing the || chain with Set.of(...).contains could look better if there are many colours that you want to check:

return balls.stream()
            .map(Ball::getColor)
            .distinct()
            .filter(Set.of("RED", "YELLOW", "GREEN")::contains)
            .limit(3)
            .count() == 3;

CodePudding user response：

Lets make it a fair fight, your original snippet is much, much longer than it needs to be:

boolean r = false, y = false, g = false;
  for (Ball ball : balls) {
    String color = ball.getColor();
    if ("RED".equals(color)) r = true;
    if ("YELLOW".equals(color)) y = true;
    if ("GREEN".equals(color)) g = true;
    if (r && y && g) return true;
  }
  return false;

Streams don't 'like it' if you have to refer to results of other operations. That's because the stream API tries to cater to way too many scenarios, thus, you get the lowest common denominator. Which, in this case, is parallel processing: Imagine java runs your stream by handing each individual item to a separated out system - now there is no longer such a thing as 'any previous result' or 'have we seen at least 1 red, at least 1 green, and at least 1 yellow ball at this point' - there is no 'this point', there's just the stream itself.

Hence, it's going to either look ugly (because you're using the wrong tool for the job), or, it's fundamentally far more inefficient. It would look something like this:

return balls.stream()
  .map(Ball::getColor)
  .filter(x -> x.equals("RED") || x.equals("GREEN") || x.equals("YELLOW"))
  .distinct()
  .count() == 3;

Comparing code lengths its not significantly simpler. It is considerably worse in performance: It needs to do a distinct scan which requires another run through, and must iterate the whole thing, whereas the first snippet will stop the moment it sees the third color.

Trying to smash those back in, you're looking at a real ugly mess. Golfed to as small as I could make it:

boolean[] c = new boolean[4];
return balls.stream()
  .map(Ball::getColor)
  .peek(x -> c[x.equals("RED") ? 0 : x.equals("YELLOW") ? 1 : x.equals("BLUE") ? 2 : 3] = true)
  .anyMatch(x -> c[0] && c[1] && c[2]);

It's not much code but it introduces all sorts of weirdness - it's weird enough that this probably needs commentary to explain what's going on. So not really a 'win'. It certainly isn't going to be any faster than the original.

In general when you are iterating over a collection with the intent to contrast between values and those operations cannot be described in terms of primitives of the list itself (such as .distinct() or .sorted() or .limit) and there is no pre-baked terminal operation (such as .max()) that does what you want, it's rather likely you do not want streams.

CodePudding user response：

You can extract distinct colors (using Stream API), then simply search in the Set.

Set<String> colors = balls.stream().map(Ball::getColor)
    .collect(Collectors.toSet());
if (colors.contains("RED") && colors.contains("GREEN") && colors.contains("YELLOW")) {
    // test passes  ...
}

If required colors are precomputed as a final Set<String>, code can be even more readable by using containsAll (checking if the retrieved set is a superset of the required set):

final Set<String> requiredColors = Set.of("RED", "GREEN", "YELLOW");
Set<String> colors = balls.stream().map(Ball::getColor)
    .collect(Collectors.toSet());
if (colors.containsAll(requiredColors)) { /* test passes */ }

CodePudding user response：

In sort my suggestions are:

Don't hard-code values to check against inside the method, provide them as a parameter.
Use enums, don't rely on strings.

Since you're describing the color of each Ball object with a string name (not for instance as a hex-code) implies that you expect only a moderate number of colors to be used in your application.

And you can improve the design of the Ball class by using a custom enum type Color instead of stream. It will guard you from making a typo and also provides a possibility to introduce a useful behavior withing the Color enum and also benefit from various language and JDK features related to enums.

public enum Color {RED, YELLOW, GREEN}

And even you don't consider utilizing enums it worth to change the method signature of the method you've listed by including an aditional parameter - a Set of colors instead of hard-coding them.

Note: there's also an inconsistency between the title and the code you've provided. The title says:

check if the list contains at list one object with one of three given

However, your code aims checks whether all given values are present.

That's how you can check whether at least one color from the given set is present, as the question title says,:

private static boolean isValidListStreams(final List<Ball> balls, Set<Color> colors) {
    return balls.stream()
        .map(Ball::getColor)
        .anyMatch(colors::contains);
}

But if you need to check if all the given colors are present, you can do it like that:

private static boolean isValidList(final List<Ball> balls, Set<Color> colors) {
    return colors.equals(
      balls.stream()
          .map(Ball::getColor)
          .filter(colors::contains)
          .limit(colors.size())
          .collect(Collectors.toSet())
    );
}

main()

public static void main(String[] args) {
    List<Ball> balls = // initializing the source list
    
    isValidListStreams(balls, Set.of(Color.RED, Color.GREEN, Color.YELLOW)); // or simply EnumSet.allOf(Color.class) when you need all enum elements instead of enumerating them
}