Vfrdtyky

I have the following method (see below). The code is working but I got сomments that there is a lot of repeating and that I should use IntStream.

Could you specify how to better optimize the code? Thanks in advance.

public static List<Integer> oddOrEven(List<Integer> integers) {

    long sum = integers.stream().mapToInt(i ->i).summaryStatistics().getSum();

    if (sum % 2 == 0) {

        return integers.stream().filter(x -> x % 2==0).distinct().collect(Collectors.toList());

    } else if (sum % 2 != 0) {

        return integers.stream().filter(x -> x % 2 != 0).distinct().collect(Collectors.toList());

    }

    return null;

}

It looks like you only need the sum of the elements in order to check if it's odd or even. To know that, it's enough to check if the number of odd elements is odd or even.

You can split the input into odd and even lists, and decide which one to return based on the size of the odd List:

public static List<Integer> oddOrEven(List<Integer> integers) {

    Map<Boolean,List<Integer>> 

        oddsAndEvens = integers.stream()

                               .collect(Collectors.partitioningBy(i->i%2==0));

    return oddsAndEvens.get(false).size() % 2 == 0 ? // check if there's an even number of odd

                                                     // elements, which means the sum is even

           oddsAndEvens.get(true) : // return the even elements

           oddsAndEvens.get(false); // return the odd elements

}

Testing:

System.out.println (oddOrEven(Arrays.asList (1,2,3,4,5)));

System.out.println (oddOrEven(Arrays.asList (1,2,3,4,5,3)));

Output:

[1, 3, 5]

[2, 4]

EDIT:

In my original answer I missed the distinct() step, which should be performed after we decide whether to return the odd or even elements. I'm afraid this requires adding a second Stream pipeline:

public static List<Integer> oddOrEven(List<Integer> integers) {

    Map<Boolean,List<Integer>> 

        oddsAndEvens = integers.stream()

                               .collect(Collectors.partitioningBy(i->i%2==0));

    return oddsAndEvens.get(false).size() % 2 == 0 ?

           oddsAndEvens.get(true).stream().distinct().collect(Collectors.toList()) :

           oddsAndEvens.get(false).stream().distinct().collect(Collectors.toList());

}

or (with Holger's suggestion):

public static List<Integer> oddOrEven(List<Integer> integers) {

    Map<Boolean,List<Integer>> 

        oddsAndEvens = integers.stream()

                               .collect(Collectors.partitioningBy(i->i%2==0));

    return oddsAndEvens.get(oddsAndEvens.get(false).size() % 2 == 0)

                       .stream()

                       .distinct()

                       .collect(Collectors.toList());

}

 long sum = integers.stream().reduce(0, (u, v) -> u + v);

 return integers.stream().filter(x -> (x % 2)==(sum % 2)).distinct().collect(Collectors.toList());

I will give you one simpler solution which uses the standard java language constructs without creating other parasite objects which streams create. I don't say streams are bad. What I am saying is that if you pass a List to a method and return a List then the conversion to stream and collecting back to list is extra work you don't need and you shouldn't do. You can try it for educational purposes and to learn streams but in this particular case you just lose performance and if you start doing it like that everywhere in real application it will add up and it can become significant problem. Even though premature optimization is not a good thing the same can be said to doing everything with streams just because they are in Java and not care about performance at all.

If we have even number of odd numbers then the sum is also even. So here is some code with bitwise operators (just for fun - you can write it with standard i % 2 etc.):

public static Collection<Integer> oddOrEven(List<Integer> integers) {

    int mod = 0;

    for (Integer i : integers) {

        if((i & 1) == 1) mod=mod^1;

    }

    Set<Integer> result = new HashSet<>();

    for (Integer i : integers){

        if (((i & 1) ^ mod)==0)     

           result.add(i);



      }

  return result; //Converting to set because we want distinct results.

                 // If the order is important we can use treeset or convert to list

}

You can do some testing against the other Stream based solutions. The solution using standard java construct without the extra overload will be the same speed or faster in most cases. For example using map is nice but with bigger list with lots of repeating numbers at some point the amount of collisions can make it many times slower than the other ways. The complexity of all algorithms is linear but the amount of work in one iteration can vary with the different stream solutions (as well as the old school approach) and if one doesn't know what is the loss there maybe sticking to the well known and easy to predict scenario is better.

public static List<Integer> oddOrEven(List<Integer> integers) {

    Function<Integer, Integer> fun =  i -> i%2;

    Map<Integer, List<Integer>> map = integers.stream().collect(Collectors.groupingBy(fun));        

    return map.get(1).size()%2 == 0? map.get(0): map.get(1);

}