Rearrange order of an ArrayList

Question

I want to rearrange an ArrayList by iterating through it and copying each element to a specific place in a new list.

In this case I want to move an element to the end of the list. For example, if the list is ABCDE and j == B then the new list should be ACDEB.

Here's my code:

private ArrayList<Job> schedule;
private ArrayList<Job> tempSchedule;

...

schedule = input;
tempSchedule = new ArrayList<Job>(schedule.size());

...

private void moveJob(int j) {
    for(int i = 0; i < schedule.size(); i++) {
        if(i == j) { //move to the end
            tempSchedule.set(schedule.size()-1, schedule.get(i));
        } else {
            if(i > j && i <= schedule.size() -1) { //move one position back
                tempSchedule.set(i - 1, schedule.get(i));
            } else { //same position
                tempSchedule.set(i, schedule.get(i));
            }
        }
    }
    schedule = tempSchedule;
    u++;
}

Right now I get an IndexOutOfBoundsException: Index: 0, Size: 0 at tempSchedule.set

I guess the problem is with this line

tempSchedule = new ArrayList<Job>(schedule.size());

Also please explain how to make deep copies.

Edit: Thanks for all the answers. I got it to run by simply removing the item and adding it at the end, like suggested.

The reason I wanted to construct a new list is because I might have to do more complex rearrangements at some point.

score 3 · Accepted Answer · answered Nov 20 '14 at 13:19

First, go read the javadoc on ArrayList and collections.

new ArrayList(capacity) doesn't copy, it just allocates a list with that capacity. To copy the list (and it's not a clone, it's a by reference copy, again you need to go back to basics) would be new ArrayList(oldArrayList).

Secondly, Your test has size 0, so there's no objects in it, so get(0) (correctly and as per spec) throws an index out of bounds exception because your list is empty.

Beyond that, neither set nor get will modify the list, so if you had created your copy correctly and it's contents were ABCD and you executed that operation, it's contents would then be ABCB. what you want is.

X = tempSchedule.remove(i) // removes element at I
tempSchedule.add(X)        // adds element to end of list

score 1 · Answer 2 · answered Nov 20 '14 at 13:16

tempSchedule is initialized to be empty:

tempSchedule = new ArrayList<Job>(schedule.size());

You can't use set on an empty ArrayList. It expects the index you are replacing to already have a value.

You get the exception in this line - tempSchedule.set(i, schedule.get(i)); - when i==0.

set calls RangeCheck :

/**
 * Checks if the given index is in range.  If not, throws an appropriate
 * runtime exception.  This method does *not* check if the index is
 * negative: It is always used immediately prior to an array access,
 * which throws an ArrayIndexOutOfBoundsException if index is negative.
 */
private void RangeCheck(int index) {
if (index >= size)
    throw new IndexOutOfBoundsException(
    "Index: "+index+", Size: "+size);
}

As you can see, the index you pass to it must be smaller than the current size of the list.

score 1 · Answer 3 · answered Nov 20 '14 at 13:16

The problem is that your tempSchedule list is empty. set() overwrites the element at the given position. If your list is empty, it can't do that.

This might be a little confusing since you wrote new ArrayList<Job>(schedule.size()). But the parameter you are passing doesn't set the size but the initial capacity, meaning the initial size of the underlying array, which can be used before it has to be resized.

score 1 · Answer 4 · answered Nov 20 '14 at 13:17

Reason is when you define arrayList with size of schedule, its an empty list i.e. contains nothing.

So when you try to set an element (which is used to replace the existing element), it compares the index with size of your list and finds that index is 0 and size is 0 as well.

Note just by passing size as constructor, you are not changing the size of arrayList. So in order to avoid this, you need to use:

tempSchedule = new ArrayList<Integer>(schedule);

instead of

tempSchedule = new ArrayList<Integer>(schedule.size());

Narmer · Answer 5 · 2014-11-20T13:26:25.810

1

You have a syntax fallacy as every other answer stated.
I'm more concerned on your approach.

Can't you just simply do:

private void moveJob(int j) {
    Job toMove = tempSchedule.get(j);
    tempSchedule.remove(j);
    tempSchedule.add(toMove);
}

Or yet more concise:

private void moveJob(int j) {
    tempSchedule.add(tempSchedule.remove(j));
}

edited Nov 20 '14 at 13:26

answered Nov 20 '14 at 13:21

Narmer

1,414
7
16

score 1 · Answer 6 · answered Nov 20 '14 at 13:21

1

You have the IndexOutOfBoundsException because you are using schedule.size in your for loop while it's null you have to use tempSchedule.size instead.

And you are comparing i and j while you have to compare tempSchedule.get(i) and j.

answered Nov 20 '14 at 13:21

cнŝdk

31,391
7
56
78

Rearrange order of an ArrayList

6 Answers6