move element to end of java array

Viewed 280

solving something in algoexpert and getting weird index out of bound exception:

the question is simply to take an array and another int and put all the numbers that equal to this int in the end of the array, like this:

array: [2, 1, 2, 2, 2, 3, 4, 2] integer toMove: 2

output: [1,3,4,2,2,2,2,2]

so I wrote this algorithm:

class Program {
  public static List<Integer> moveElementToEnd(List<Integer> array, int toMove) {
    // Write your code here.
        int newArr[] = new int[array.size()];
        int lastIndertedNotToMoveIdx = 0;
        int endOfArrayIdx = array.size();
        // ArrayList<Integer> list = new ArrayList<Integer>();
        for (int i = 0; i <= array.size(); i++) {
            if (array.get(i) == toMove) {
                newArr[endOfArrayIdx] = array.get(i);
                    endOfArrayIdx = endOfArrayIdx-1;
            } else {
                newArr[lastIndertedNotToMoveIdx] = array.get(i);
                lastIndertedNotToMoveIdx++;
            }
        }
                
      List<Integer> ret_list = new ArrayList<Integer>();
      for(Integer num:newArr) {
         ret_list.add(num);
      }
    return ret_list;
  }
}

but here newArr[endOfArrayIdx] = array.get(i); I get this message:

java.lang.ArrayIndexOutOfBoundsException: Index 8 out of bounds for length 8
    at Program.moveElementToEnd(Program.java:12)
    at AeJsonTest.getActual(AeJsonTest.java:27)
    at Main.main(Main.java:57)

not sure why...

3 Answers

endOfArrayIdx should start at array.size() - 1, rather than array.size(), which is the array's length. The array's length is always an invalid index for the array.

Also, the for loop condition should be i < array.size(), not i <= array.size().

newArr[endOfArrayIdx] = array.get(i);

should be:

newArr[endOfArrayIdx - 1] = array.get(i);

as arrays in java go from 0 to length - 1.

Since you have a list, use it! It'll be much easier than what you do. And your risk of "weird IndexOutOfBoundException" will be reduced.

  static List<Integer> moveElementToEnd(List<Integer> array, int toMove) {
    for (int i = 0, lastElementIndex = array.size() - 1; i <= lastElementIndex; ) {
      if (array.get(i) == toMove) {
        array.remove(i);
        array.add(toMove);
        lastElementIndex--;
      } else {
        i++;
      }
    }
    return array;
  }

An alternative would be to not touch the input list at all. Here you don't even have any index!

  static List<Integer> moveElementToEnd(List<Integer> array, int toMove) {
    List<Integer> head = new ArrayList<>();
    int toMoveCount = 0;
    for (int value : array) {
      if (value == toMove) {
        toMoveCount++;
      } else {
        head.add(value);
      }
    }
    head.addAll(Collections.nCopies(toMoveCount, toMove));
    return head;
  }
Related