[FIXED] Strange bug with Streams

Issue

So I have created my own Set, which is just a regular set, but has additional functions (for example my set only stores absolute values).

Here is my Code:

import java.util.*;

public class SortedByAbsoluteValueIntegerSet<E> extends HashSet<E> {

private Set<Integer> mySet;

public SortedByAbsoluteValueIntegerSet() {
    mySet = new HashSet<Integer>();
}

@Override
public int size() {
    return mySet.size();
}
@Override
public boolean add(E e){
    return mySet.add(Math.abs((Integer) e));
}


@Override
public boolean remove(Object o) {
    return mySet.remove(o);
}

@Override
public boolean contains(Object o){
    return mySet.contains(o);
}

@Override
public boolean addAll(Collection<? extends E> c) {
    List<Integer> myList = new ArrayList<>();
    for (Object e: c) {
        myList.add(Math.abs((Integer) e));
    }
    return mySet.addAll(myList);
}

@Override
public String toString(){
    return mySet.toString();
}

}

I had a test case in JUnit, which failed. Because there was some issue with my code. For demonstration purpose, and for me to explain my issue better I have created two functions, which show the problem well.

Here is the problem:

public static void testSortedByAbsoluteValueIntegerSet() {
    Set<Integer> set1 = new SortedByAbsoluteValueIntegerSet();
    Set<Integer> set2 = new HashSet<>();
    set1.add(5);
    set1.add(3);
    set2.add(5);
    set2.add(3);
    String x = toString(set1); //x is  ""
    String t = toString(set2); //t is "3 5"
}

public static String toString(final Collection<Integer> collection) {
    return String.join(" ", collection.stream()
            .map(i -> Integer.toString(i))
            .toArray(String[]::new));
}

So the problem arises in this line:

String x = toString(set1); //x is always an empty string
String t = toString(set2); //t works correctly

When I go through debugger I see that String x is always an empty String and String t works correctly. By the way set1 is representation of my created set and set2 is just a regular hashset.

The question is: how can I fix my SortedByAbsoluteValueIntegerSet class so that the toString() method worked fine with my own created set as well.

P.S I am new to streams and I don't really understand the problem, why does it happens.

Solution

I think Tomas F gave better answer

Main problem in your set is using HashSet mySet as field and extending HashSet. In java better to use (field) composition instead of extending to add some functionality to your class. Here you tried use both - it's not a good idea. Best decision is to use just composition and extending more general class, for example AbstractSet<Integer> and Set<Integer>:

import java.util.*;

public class SortedByAbsoluteValueIntegerSet extends AbstractSet<Integer> 
        implements Set<Integer>, java.io.Serializable {

    private final Set<Integer> mySet;

    public SortedByAbsoluteValueIntegerSet() {
        mySet = new HashSet<>();
    }

    @Override
    public Iterator<Integer> iterator() {
        return mySet.iterator();
    }

    @Override
    public int size() {
        return mySet.size();
    }

    @Override
    public boolean add(Integer e) {
        return mySet.add(Math.abs(e));
    }


    @Override
    public boolean remove(Object o) {
        return mySet.remove(o);
    }

    @Override
    public boolean contains(Object o) {
        return mySet.contains(o);
    }

    @Override
    public boolean addAll(Collection<? extends Integer> c) {
        List<Integer> myList = new ArrayList<>();
        for (Integer e : c) {
            myList.add(Math.abs(e));
        }
        return mySet.addAll(myList);
    }

    @Override
    public String toString() {
        return mySet.toString();
    }
}

in this case you don't have to implement spliterator, because Set has default implementation using this keyword (which is refer to your set as a Collection)

but also you can implement spliterator in your class (but using such extends and internal Set fields are the bad practice. Also, it's better to get rid of type parameter E and casting elements to Integer:

import java.util.*;

public class SortedByAbsoluteValueIntegerSet extends HashSet<Integer> {

    private Set<Integer> mySet;

    public SortedByAbsoluteValueIntegerSet() {
        mySet = new HashSet<>();
    }

    @Override
    public int size() {
        return mySet.size();
    }
    @Override
    public boolean add(Integer e){
        return mySet.add(Math.abs(e));
    }


    @Override
    public boolean remove(Object o) {
        return mySet.remove(o);
    }

    @Override
    public boolean contains(Object o){
        return mySet.contains(o);
    }

    @Override
    public boolean addAll(Collection<? extends Integer> c) {
        List<Integer> myList = new ArrayList<>();
        for (Integer e: c) {
            myList.add(Math.abs(e));
        }
        return mySet.addAll(myList);
    }

    @Override
    public String toString(){
        return mySet.toString();
    }

    @Override
    public Spliterator<Integer> spliterator() {
        return mySet.spliterator();
    }

}

Answered By - Boris Azanov
Answer Checked By - David Goodson (JavaFixing Volunteer)