Jack Smith
Jack Smith

Reputation: 53

Java array sorting (What am I doing wrong)

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]

Upvotes: 2

Views: 137

Answers (4)

Boliang Tan
Boliang Tan

Reputation: 1

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.

Upvotes: 0

ABHIJITH GM
ABHIJITH GM

Reputation: 134

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

Upvotes: 0

Ousmane D.
Ousmane D.

Reputation: 56423

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);
}

Upvotes: 0

vikingsteve
vikingsteve

Reputation: 40378

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.

Upvotes: 1

Related Questions