Hello everyone!
I'm currently making my own implementation of java.util.List, and it's going pretty well.
I've just run into a minor problem.
I'm currently doing the 'addAll(int index, Collection<?> c);' method.
But I can't seem to figure out how to make it start at the specified index.. this is the method so far:

@Override
    public boolean addAll(int index, Collection<? extends E> c){
        for(int i = index; i < c.size(); i++){
            if(!add(c.iterator().next())){
                return false;
            }else{
                continue;
            }
        }

        return true;
    }

And this is the output of an implementation looking like this:
Implementation.

NodeList<Integer> nodeList = new NodeList<Integer>(10);
        List<Integer> list = new ArrayList<Integer>();

        list.add(10);
        list.add(20);
        list.add(56);
        list.add(45);
        nodeList.addAll(2, list);

        for(int i = 0; i < nodeList.size(); i++){
            System.out.println(nodeList.get(i));
        }

Output.

10
10

Please help me :).

Recommended Answers

All 9 Replies

Hint: c.iterator() creates a new Iterator instance.

Also, calling next() on an Iterator without testing the presence of an element using hasNext() is not recommended.

Hint: c.iterator() creates a new Iterator instance.

Also, calling next() on an Iterator without testing the presence of an element using hasNext() is not recommended.

Alright.. I changed the method to this:

@Override
    public boolean addAll(int index, Collection<? extends E> c){
        for(int i = 0; i < c.size(); i++){
            for(Iterator iterator = c.iterator(); iterator.hasNext();){
                if(!add((E)iterator.next())){
                    return false;
                }
            }
        }

        return true;
    }

And now the output with the same implementation is:

10
20
56
45
10
20
56
45
10
20

The problem is that you never use the `index' parameter in your overridden method. Also, you can avoid the cast by using the iterator as Iterator<? extends E> .

The problem is that you never use the `index' parameter in your overridden method. Also, you can avoid the cast by using the iterator as Iterator<? extends E> .

Sorry that was a typo.
This is the real method:

@Override
    public boolean addAll(int index, Collection<? extends E> c){
        for(int i = index; i < c.size(); i++){
            for(Iterator<? extends E> iterator = c.iterator(); iterator.
                    hasNext();){
                if(!add(iterator.next())){
                    return false;
                }
            }
        }

        return true;
    }

And this is the implentation:

NodeList<Integer> nodeList = new NodeList<Integer>(10);
        List<Integer> list = new ArrayList<Integer>();

        list.add(10);
        list.add(20);
        list.add(56);
        list.add(45);
        nodeList.addAll(2, list);
        
        for(int i = 0; i < nodeList.size(); i++){
            System.out.println(nodeList.get(i));
        }

Yet, my output is:

10
20
56
45
10
20
56
45

Think about what you've written for a while; do you really need two looping structures just to loop over a subset of a collection?

Given that Collection interface doesn't have a get method, the only option which remains is to keep an external counter which is incremented inside the iterator loop. This counter would keep track of the elements which need to be skipped (by comparing the counter value with the `index' passed in).

Oh and BTW, if this requirement is plain wrong/illogical. There is a reason why the Collection interface doesn't have a get method...

Think about what you've written for a while; do you really need two looping structures just to loop over a subset of a collection?

Given that Collection interface doesn't have a get method, the only option which remains is to keep an external counter which is incremented inside the iterator loop. This counter would keep track of the elements which need to be skipped (by comparing the counter value with the `index' passed in).

Oh and BTW, if this requirement is plain wrong/illogical. There is a reason why the Collection interface doesn't have a get method...

This method:

@Override
    public boolean addAll(int index, Collection<? extends E> c){
        int i = 0;

        for(Iterator<? extends E> iterator = c.iterator(); iterator.
                hasNext();){
            if(i >= index){
                if(!add(iterator.next())){
                    return false;
                }
            }

            i++;
        }

        return true;
    }

Produces the following no matter what value I put in as 'index':

10
20
56
100

Please help :(.

I fixed it with this method:

@Override
    public boolean addAll(int index, Collection<? extends E> c){
        if(index == 0){
            return false;
        }

        int i = 0;
        Object next = null;

        for(Iterator <? extends E> iterator = c.iterator(); iterator.
                hasNext();){
            if(i >= index){
                if(!add((E)next)){
                    return false;
                }
            }

            i++;
            next = iterator.next();
        }

        if(!add((E)next)){
            return false;
        }

        return true;
    }

The problem with your previous implementation was that the invocation of next() was inside a condition which shouldn't have been the case. Also, you can again avoid casts by declaring next as E next = null; .

Again, like I said, making the method accept a Collection is plain wrong since a Collection doesn't support the notion or is not aware of indexed/ordered access. Your custom list implementation should accept only a List which would make your class semantically correct.

Anyways, here is my stab at it:

public class LinkedListTest {

    public static void main(final String[] args) {
        testNormal();
        testEmpty();
        testNull();
    }

    private static void testNormal() {
        List<Integer> list = new ArrayList<Integer>();
        list.add(10);   list.add(20);  list.add(30);  list.add(40);
        NodeList<Integer> nodeList = new NodeList<Integer>();
        nodeList.add(1);
        nodeList.addAll(2, list);
        System.out.println(nodeList);
    }

    private static void testNull() {
        NodeList<Integer> nodeList = new NodeList<Integer>();
        nodeList.add(1);
        nodeList.addAll(2, null);
        System.out.println(nodeList);
    }

    private static void testEmpty() {
        List<Integer> list = new ArrayList<Integer>();
        NodeList<Integer> nodeList = new NodeList<Integer>();
        nodeList.add(1);
        nodeList.addAll(2, list);
        System.out.println(nodeList);
    }

}

class NodeList<E> extends LinkedList<E> {

    private static final long serialVersionUID = 1L;

    public boolean addAll(final int startIdx, final Collection<? extends E> collection) {
        if (collection == null || collection.isEmpty()) {
            return false;
        }

        final int size = collection.size();
        if (startIdx < 0 || startIdx > size - 1) {
            throw new IndexOutOfBoundsException(
                    "An out of bounds index specified: " + startIdx);
        }

        int i = 0;
        boolean collectionChanged = false;
        for (Iterator<? extends E> iter = collection.iterator(); iter.hasNext(); ++i) {
            E elem = iter.next();
            if (i >= startIdx) {
                collectionChanged = true;
                add(elem);
            }
        }
        return collectionChanged;
    }

}
commented: Very helpful feedback +4
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.