Inserting elements into array

Question

I have a class that implements an interface and I think when I try to insert an element into the array more than once the first insertions are forgotten. I really can't figure this one out. This is what I have:

public void insertElementAt(int index, E el)
                 throws IllegalArgumentException {

    Object temp[] = new Object[data.length + 1];
    for (int i = 0; i < data.length; i++) {
        if (i == index){
            temp[index] = el;
            temp[i + 1] = data[i];
            i++;
        }

        temp[i] = data[i];
    }

    data = temp;

    if (index > data.length || index < 0) {
        throw new IllegalArgumentException();
    }
}

Then my test reports null instead of first at he last assertion.

@Test
public void testInsertToLeft() {
    PriorityList<String> list = new ArrayPriorityList<String>();
    list.insertElementAt(0, "First");
    // Must shift array elements in this case
    list.insertElementAt(0, "New First");

    assertEquals("New First", list.getElementAt(0));
    assertEquals("First", list.getElementAt(1));
}

Looks like you need two indexes - one for the old array and one for the new array. As it stands, when you skip along one element because of the insert you skip along both arrays. P.S. just use a List. — Boris the Spider
– Boris the Spider, Commented Oct 8, 2013 at 20:31

angel_navarro · Accepted Answer · 2013-10-08 21:01:24Z

1

You must do like this:

public void insertElementAt(int index, E el) throws IllegalArgumentException {

    Object temp[] = new Object[data.length + 1];
    for (int i = 0; i < data.length; i++) {
        if (i >= index){
            temp[i + 1] = data[i];
        } else {
            temp[i] = data[i];
        }
    }
    temp[index] = el;
    data = temp;

    if (index > data.length || index < 0) {
        throw new IllegalArgumentException();
    }
}

To remove it:

public void removeElementAt(int index) throws IllegalArgumentException {

    Object temp[] = new Object[data.length - 1];
    for (int i = 0; i < temp.length; i++) {
        if (i > index){
            temp[i - 1] = data[i];
        } else {
            temp[i] = data[i];
        }
    }
    data = temp;

    if (index > data.length || index < 0) {
        throw new IllegalArgumentException();
    }
}

edited Oct 8, 2013 at 21:01

answered Oct 8, 2013 at 20:39

angel_navarro

1,77710 silver badges11 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Kevin DiTraglia Over a year ago

I figured there was an easy fix without changing too much of his existing code (But I couldn't seem to figure it out), nice answer +1.

Tejas Over a year ago

perfect. would I do something similar to remove an element out of the array and shift it down?

Tejas Over a year ago

you are awesome. does this similar concept apply when taking an element out and moving it to the end and shifting everything down? also taking an element out and putting it at the start and moving everything up?

angel_navarro Over a year ago

It would be something, but considering that in this case both arrays keep the same size ;-)

angel_navarro Over a year ago

@Tejas, If my answer has been helpful to you, please do not forget to mark it as correct. This may be useful for other developers ;-) Thanks!

|

jama · Accepted Answer · 2013-10-08 20:36:26Z

0

Try changing your for loop to either:

for (int i = 0; i < data.length; i++) {
            if (i == index){
                temp[index] = el;
                temp[i + 1] = data[i];
                i++;
            }else{
                temp[i] = data[i];
            }

}

or

for (int i = 0; i < data.length; i++) {
        if (i == index){
            temp[index] = el;
            temp[i + 1] = data[i];
            i++;
            continue;
        }
        temp[i] = data[i];
    }

answered Oct 8, 2013 at 20:36

jama

3251 silver badge6 bronze badges

Comments

arynaq · Accepted Answer · 2013-10-08 20:38:23Z

0

What is data.length when the list is empty? If it is empty at first insertion you will not enter the for loop but copy the temp array, it will go into the loop at next insertion with length 1. The first insertion will be skipped.

answered Oct 8, 2013 at 20:38

arynaq

6,8709 gold badges49 silver badges77 bronze badges

Comments

Irwin · Accepted Answer · 2013-10-08 20:48:48Z

0

I would do this:

public static void insertElementAt(int index, E el)
    throws IllegalArgumentException {

if (index > data.length || index < 0) {
    throw new IllegalArgumentException();
}

Object temp[] = new Object[data.length + 1];
for (int i = index; i < data.length; i++) {
    temp[i+1] = data[i];
}
temp[index] = el;
data = temp;

}

answered Oct 8, 2013 at 20:48

Irwin

12.9k12 gold badges73 silver badges97 bronze badges

Comments

Bohemian · Accepted Answer · 2013-10-08 21:40:43Z

0

You should have the test for valid parameters first ("fail early"), and you can make good use of the JDK's utility methods do the lifting for you:

public static void insertElementAt(int index, E el) {
    if (index > data.length || index < 0) {
        throw new IllegalArgumentException();
    }

    data = Arrays.copyOf(data, data.length + 1);
    System.arrayCopy(data, index, data, index + 1, data.length - index);
    data[index] = el;
}

Also note you don't need to declare a throws, becauseIllegalArgumentException is an unchecked exception, so I removed it. Normally, one follows this pattern.

edited Oct 8, 2013 at 21:40

answered Oct 8, 2013 at 20:49

Bohemian♦

427k103 gold badges603 silver badges750 bronze badges

Collectives™ on Stack Overflow

Inserting elements into array

5 Answers 5

7 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

7 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related