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]

It was used in the old method too, but for some reason the new method messes up, why is that, btw its removing'i' the subject of the loop, its not removing the same element again and again — Jack Smith
– Jack Smith, Commented Feb 16, 2017 at 13:22
To elaborate on what @vikingsteve is saying; you can another integer list that will store the values of i for which list.get(i) == item is true then iterator on the new list and remove all i's in list — Laazo
– Laazo, Commented Feb 16, 2017 at 13:24
Show the full stack trace, as well as your testing code please. — Jorn Vernee
– Jorn Vernee, Commented Feb 16, 2017 at 13:28

Community · Accepted Answer · 2017-05-23 10:29:43Z

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.

edited May 23, 2017 at 10:29

CommunityBot

11 silver badge

answered Feb 16, 2017 at 13:24

vikingsteve

40.7k25 gold badges120 silver badges164 bronze badges

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

Comments

Ousmane D. · Accepted Answer · 2017-02-16 13:25:52Z

0

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

answered Feb 16, 2017 at 13:25

Ousmane D.

56.6k8 gold badges101 silver badges134 bronze badges

8 Comments

Jack Smith Over a year ago

That method is still giving java.lang.ArrayIndexOutOfBoundsException

Ousmane D. Over a year ago

Give me a second, I'll check that with debugger

Jack Smith Over a year ago

This particular method for some reason doesnt remove the element but makes the element = null, which then causes array out of bounds because at the same time, externally i am minusing the variable that edits the master array size

Ousmane D. Over a year ago

can you provide me the arguments you're passing into the methods, the full code if possible please

Jack Smith Over a year ago

The method correctly sorts the array but doesnt -1 the size, therefore the final element is = null, causing the array out of bounds

|

ABHIJITH GM · Accepted Answer · 2017-02-16 13:26:57Z

0

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

answered Feb 16, 2017 at 13:26

ABHIJITH GM

1344 bronze badges

Comments

Boliang Tan · Accepted Answer · 2017-02-25 14:48:47Z

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.

Collectives™ on Stack Overflow

Java array sorting (What am I doing wrong)

4 Answers 4

Comments

8 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

8 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related