Removing negative values from Set

Viewed 2127

I expected remove() method removes negative values and prints only positive integers, but output is -1 -2 -3 4

 import java.util.HashSet;
    import java.util.Iterator;
    import java.util.Set;

    public class GenericSetTester {
        public static void main(String[] args) {

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

            integerSet.add(0);
            integerSet.add(1);
            integerSet.add(2);
            integerSet.add(3);
            integerSet.add(4);
            integerSet.add(-1);
            integerSet.add(-2);
            integerSet.add(-3);

            for(Iterator<Integer> in = integerSet.iterator(); in.hasNext();){
                if(in.next() < 0)
                    in.remove();
                   int i = in.next();
                System.out.printf("%d ",i);
            }
        }
    }
4 Answers

The @Eran answer corrects your current approach. Note that streams provide another way to handle your requirement:

Integer[] array = new Integer[] {0, 1, 2, 3, 4, -1, -2, -3};
Set<Integer> integerSet = new HashSet<>(Arrays.asList(array));
integerSet = integerSet.stream()
    .filter(i -> i >= 0)
    .collect(Collectors.toSet());

This loop consumes two elements of the Set on each iteration (since you call in.next() twice in each iteration), which is wrong, and leads to the negative elements not being removed.

Consider the order in which the elements of your Set are iterated:

0 // consumed by if(in.next() < 0), which returns false, element not removed
-1 // consumed by int i = in.next();, and later printed -1
1 // consumed by if(in.next() < 0), which returns false, element not removed
-2 // consumed by int i = in.next();, and later printed -2
2 // consumed by if(in.next() < 0), which returns false, element not removed
-3 // consumed by int i = in.next();, and later printed -3
3 // consumed by if(in.next() < 0), which returns false, element not removed
4 // consumed by int i = in.next();, and later printed 4

As you can see, your loop doesn't remove any element from the Set, and prints only half of the elements (-1 -2 -3 4).

It should be:

for (Iterator<Integer> in = integerSet.iterator(); in.hasNext();) {
    int i = in.next();
    if (i < 0)
        in.remove();
    else
        System.out.printf("%d ",i);
}

As Tim suggested, if you print the Set after the loop, you'll see whether or not it contains only the elements it should contain. Your loop will become:

for(Iterator<Integer> in = integerSet.iterator(); in.hasNext();){
    if(in.next() < 0)
        in.remove();
}
System.out.println (integerSet);

What is happening is the following: The integerSet adds the numbers in this order (I don't know why though): 0: 0 1: -1 2: 1 3: -2 4: 2 5: -3 6: 3 7: 4

That why if you do the loop, it first checks if the next of 0 is smaller then 0. Which will return true since -1 is smaller dan 0. So 0 will be deleted. Then it check is the next of -1 which is 1, is smaller then 0. Which will return false. So -1 will not be removed.

That why the outpput is -1 -2 -3 4

a lambda that retains the original Set

integerSet.removeAll( integerSet.stream().filter( i -> i < 0 ).collect( Collectors.toList() ) );
…

…or simple: Holger's solution – see comment below

integerSet.removeIf( i -> i < 0 );
Related