tags:

views:

44

answers:

4

I design new IntSet Class that use ArrayList. first, i extends Intset by ArrayList and i start implement method. i face some problem in my union() method. here is my code...

public class IntSet extends ArrayList<Integer>{

    private static final long serialVersionUID = 1L;
    private ArrayList<Integer> intset;

    public IntSet(){
        this.intset = new ArrayList<Integer>();
    }
    public IntSet(ArrayList<Integer> intset){
        this.intset = intset;
    }

    public void insert(int x){
        this.intset.add(x);
    }

    @Override
    public Integer remove(int x){
        int index = intset.indexOf(x);
        this.intset.remove(index);
        return 1;
    }

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

    @Override
    public Integer get(int index){
        return this.intset.get(index);
    }

    public boolean member(int x){
        if(intset.indexOf(x)==-1) return false;
        else return true;
    }

    public IntSet union(IntSet a){
        IntSet intersectSet = new IntSet();
        intersectSet.insert(0);
        intersectSet.insert(1);
        System.out.println(intersectSet.size());
        System.out.println(intersectSet.contains(1));
        for(int i=0; i<a.size(); i++){
        }
        return intersectSet;
    }

    public String toString(){
        if(intset.size()==0) return "[]";
        String s = "[" + intset.get(0).toString();
        for(int i=1; i<intset.size(); i++){
            s += "," + intset.get(i).toString();
        }
        return s += "]";
    }

}

In method

union(IntSet a);

I constract new Intset object then add 2 value (0, 1) into intersectSet variable.

intersectSet.insert(0);
intersectSet.insert(1);

then i print size of intersectSet it shown me 2 that is correct!

but when i need to check that there is 1 in intersectSet or not? it shown me fasle.

System.out.println(intersectSet.contains(1));

In fact it should show me true because in intersectSet have interger 1.

anything wrong about my code and should i extends ArrayList for IntSet class?

A: 

You are both extending ArrayList and managing your own internal ArrayList object, this means that for all the methods which you have overridden you are interacting with your intset member variable, otherwise you are interacting with the inherited internal representation used by the ArrayList superclass. If you override the contains method you will get the correct behaviour.

I suggest that you drop the subclassing of ArrayList and instead implement the List or Set interfaces, although this depends on the exact problem you've been asked to solve.

Jon Freedman
A: 

you need to override contains method.

public boolean contains(Object o) {
    return intset.contains(o);
    }

and the rest of ArrayList methods that related to its elements.

and i doesn't seems to me a good solution. you may try better approach.

mohammad shamsi
+1  A: 

Some suggestions on the class design:

  • Don't have your class extend ArrayList. A "set" really shouldn't be extending List. However, you should probably implement Set. This will have the added bonus of the compiler telling you what methods you need to implement for a set.....
  • For fastest performance (but more work!), you may want to use an internal array rather than an ArrayList.
  • Consider making the structure immutable, with functions that return a new copy rather than mutating the set in place. Depending on your usage, this may be a better solution, especially if you are mostly dealing with small, non-changing sets.
  • Again depending on your usage, you may want to override hashCode and equals to implement value based equality
  • When you construct the Intset with an ArrayList, you should ideally defensively copy (clone) the ArrayList. You don't want you set to change if someone mutates the original ArrayList.
mikera
+1  A: 

The problem here is that you actually have 2 ArrayLists. The IntSet class IS A ArrayList, but this class contains a second ArrayList intset. Get rid of one of these ArrayLists. To demonstrate this add this second line:

System.out.println(intersectSet.contains(1));
System.out.println(intersectSet.intset.contains(1));

this will output:

false
true

So you are going to have to make a choice, do I inherit from ArrayList or do I contain an ArrayList. Of course what I am getting at here is Item 16 from Effective Java, Favor composition over inheritance.

krock
@krock how do i change constructor? give me an idea.
Giffary
@Giffary, You can keep the constructor the way it is (use composition), but if you do this you shouldn't extend ArrayList as well. Instead, implement the List interface. Bloch does this in his `Set` example by not extending the concrete class `HashSet` but by implementing the interface itself, by doing this with `List` you are no longer burdened by the implementation of `ArrayList` and your instrumentation of the `List` is decoupled from the implementation of the List.
krock