Java array sorting (What am I doing wrong)

Question

EDIT 3: this is the cause of the problem:

For removeArrayElement (first version), its returning toArray(new Item[0]), which executes the null element at the end, but with the new method, it returns toArray(arr), which does not execute the null, but you can;t create a generic type array of T, i.e new T[0], so what is a substitute? instead of 'passing the array again' to get rid of the null element at the end

Old problem:

I recently updated how I was handling array sorting by creating one master method (by implements generic types) Only the aftermath method is giving me array out of bounds errors.

Is there anything I've missed?

Old methods:

private static Item[] insertTabItem(Item[] a, int pos, Item item) {
    Item[] result = new Item[a.length + 1];
    for(int i = 0; i < pos; i++)
        result[i] = a[i];
    result[pos] = item;
    for(int i = pos + 1; i < a.length + 1; i++)
        result[i] = a[i - 1];
    return result;
}

private static Item[] removeArrayItem(Item[] arr, Item item) {
   List<Item> list = new ArrayList<Item>(Arrays.asList(arr));
      for (int i = 0; i < list.size(); i++) {
          if (list.get(i) == item) {
              list.remove(i);
          }
      }
     return list.toArray(new Item[0]);
}

new methods (giving java.lang.ArrayIndexOutOfBoundsException)

public static <T> T[] insertArrayElement(T[] arr, int pos, T item) {
     final int N = arr.length;
    T[] result = Arrays.copyOf(arr, N + 1);
    for(int i = 0; i < pos; i++)
        result[i] = arr[i];
    result[pos] = item;
    for(int i = pos + 1; i < N + 1; i++)
        result[i] = arr[i - 1];
    return result;
}

public static <T> T[] removeArrayElement(T[] arr, T item) {
   List<T> list = new ArrayList<T>(Arrays.asList(arr));
      for (int i = 0; i < list.size(); i++) {
          if (list.get(i) == item) {
              list.remove(i);
          }
      }
     return list.toArray(arr);
}

EDIT:

After reading through some of the answer I've changed the removeArrayElement to this:

public static <T> T[] removeArrayElement(T[] arr, T item) {
   for (Iterator<T> iterator = list.iterator(); iterator.hasNext();) {
       T t = iterator.next();
        if (t == item) {
            iterator.remove();
        }
    }
     return list.toArray(arr);
}

but it still for some reason sends this: java.lang.ArrayIndexOutOfBoundsException

EDIT2: Full executable

bankContents[bankSlots[0]] = Utils.removeArrayElement(bankContents[bankSlots[0]], newbankVarient);

When newBankVarient is = bankContents[bankSlots[0]][1] and removing it, System out of array AFTER is:

"[var, var, var, null]

Answer 1

Without running your code it seems likely that removeArrayElement is causing the exception.

It's bad practise to use list.remove(i) from within a for loop that iterates the same loop.

Either, you need to break; directly after the remove(i) call, or you can consider using an iterator instead, which is "safe to delete whilst iterating".

For example: Java, Using Iterator to search an ArrayList and delete matching objects

Lastly if T is comparable then you should be able to delete from a list via list.remove(item) - you don't need to mess around with indexes if you dont need to.

Answer 2

as @vikingsteve has mentioned don't use remove method inside a loop, as you're most likely to jump ahead of the possible indexes.

however, you can keep the loop and start looping from the back rather than from the front.

something like this would do:

public static <T> T[] removeArrayElement(T[] arr, T item) {
   List<T> list = new ArrayList<T>(Arrays.asList(arr));
      for (int i = list.size()-1; i >= 0; i--) {
          if (list.get(i) == item) {
              list.remove(i);
          }
      }
     return list.toArray(arr);
}

Answer 3

remove inside a loop is the problem.eitger decrement i by 1 (i --) when an item is removed or use iterator based loop and call iterator.remove

Answer 4

For your EDIT 3, it's the correct behaviour described in the javadoc

If the list fits in the specified array with room to spare (i.e., the array has more elements than the list), the element in the array immediately following the end of the list is set to null. (This is useful in determining the length of the list only if the caller knows that the list does not contain any null elements.)

Keep the type with an empty copy of your array

private static Item[] removeArrayItem(Item[] arr, Item item) {
    Item[] emptyArray = Arrays.copyOf(arr, 0);
    List<Item> list = new ArrayList<Item>(Arrays.asList(arr));
    for (Iterator<Item> iter = list.iterator(); iter.hasNext();) {
      Item current = iter.next();
      if (item.equals(current)) {
        iter.remove();
        break;
      }
    }
    return list.toArray(emptyArray);
}

Btw, equality test on ref is a bad pratice, prefer the equals when you can. Maybe you can also, if your items are comparable, use the Arrays.binarySearch to get the index of the item you want to remove avoiding iterator in favor of a direct use remove(index) on the list.