0

I want to remove an index from an array and have tried to do so by copying over all the elements except the one that I want to remove into a new array, however my solution gets ArrayIndexOutOfBoundsException, what should I do to solve this?

public void removeItem(Item item) {
    Item[] ownedItemCopy = new Item[ownedItem.length - 1];
    for (int i = 0; i < ownedItem.length; i++) {
        if (!ownedItem[i].equals(item)) {
            int j = i;
            ownedItemCopy[j - 1] = ownedItem[i];
        }
    }
    ownedItem = ownedItemCopy;
}
Daniel Widdis
  • 8,424
  • 13
  • 41
  • 63
tewi
  • 21
  • 4
  • 1
    Well, the first thing you should try is to use an `ArrayList` instead. – markspace Feb 23 '21 at 15:25
  • 1
    What is `j` in the first iteration of the loop? Then what is `j-1` and is that a valid index? – Tom Feb 23 '21 at 15:26
  • @Tom that was just a temporary solution since I first had ownedItemCopy[i]=ownedItem[I]; but I figured that since the new array needs to be smaller than the original one so I used 2 variables, it worked in som test cases but in some it still got out of bounds – tewi Feb 23 '21 at 15:32

5 Answers5

1

Since you are looping in two arrays that are not same lenght, you need two indexes. BUT you need to increment the "shorter array" index only if an item is added to it. It should be something like this

public void removeItem(Item item) {

    if(ownedItem.length == 0) return;  //guard against problems if array is empty
    else if (!Arrays.asList(ownedItem).contains(item)) return;  //guard against problems if array does not contain requested item
    
    Item[] ownedItemCopy  = new Item [ownedItem.length - 1 ];
    
    for (int i = 0, j=0; i < ownedItem.length; i++) { 
            
       if(ownedItem[i].equals(item)) continue;
 
       ownedItemCopy[j++] = ownedItem[i];
    }
    ownedItem=ownedItemCopy;
} 
Siberio
  • 105
  • 9
  • 1
    Nearly. You must first check, if the value You want to delete is in the array. Otherwise You get another `ArrayIndexOutOfBoundsException` because Your `ownedItemCopy` is too short. – Kaplan Feb 23 '21 at 17:10
  • You are right! @Kaplan I will add a guard for this case – Siberio Feb 25 '21 at 08:47
0

When i == 0 you assign j=i, making j equal to 0, then you call ownedItemCopy[j-1], thus calling ownedItemCopy[-1].

To solve this, the if statement checking for 'is not equal to' should be changed to checking for the case when it is equal to and when this is true execution should continue to the next iteration and thereby skipping the rest of the current iteration.

public void removeItem(Item item) {

    Item[] ownedItemCopy  = new Item [ownedItem.length - 1];
           
       for (int i = 0, j = 0; i < ownedItem.length; i++) { 
                
            if(ownedItem[i].equals(item)) { continue; } 

                ownedItemCopy[j++]=ownedItem[i];
       }

       ownedItem=ownedItemCopy;
}
  • Oh yeah that's right, do you have any suggestions on how I can avoid ArrayIndexOutOfBoundsException? – tewi Feb 23 '21 at 15:35
  • 1
    'public void removeItem(Item item) { Item[] ownedItemCopy = new Item [ownedItem.length - 1 ]; for (int i = 0; i < ownedItem.length; i++) { if(ownedItem[i].equals(item)) { continue; } ownedItemCopy[j++]=ownedItem[i]; } ownedItem=ownedItemCopy; }' – Jens van Groeningen Feb 23 '21 at 15:42
  • 1
    Basically, you should replace the `if(!ownedItem[i].equals(item))` with: `if(ownedItem[i].equals(item))` and inside of this if statement you should continue, causing it to skip to the next iteration. And inside the loop you should just execute: `ownedItemCopy[j++]=ownedItem[i];` – Jens van Groeningen Feb 23 '21 at 15:45
  • I get your thought, however, j isn't declared anywhere? – tewi Feb 23 '21 at 15:55
  • and if I put int j = i; this only works if there is only one element in the array, if there are more elements in the array it gets out of bounds... – tewi Feb 23 '21 at 15:59
  • 1
    I think you can best declare j in the initialization section of the for-loop, like this: `for (int i = 0, j = 0; i < ownedItem.length; i++)` – Jens van Groeningen Feb 23 '21 at 16:01
  • No problem. Glad I could help! – Jens van Groeningen Feb 23 '21 at 16:11
0

There are some logical issues.

  • You want to remove an item from your array and want to create a new array. So, your new array's length is not equal to your old array's length.

  • Your for loop should iterate i < ownedItem.length - 1 instead of i < ownedItem.length

  • When you find item in the array, then you should start shifting. Here I use a boolean key for this.

public void removeItem(Item item) {
    boolean canStartShifting = false;

    Item[] ownedItemCopy = new Item[ownedItem.length - 1];

    for (int i = 0; i < ownedItem.length - 1; i++) {

        if (ownedItem[i].equals(item)) {
            canStartShifting = true;
        }

        if (canStartShifting) {
            ownedItemCopy[i] = ownedItem[i + 1];
        }
    }
}
Md Kawser Habib
  • 1,966
  • 2
  • 10
  • 25
0

If i=j=0, then j-1=-1, which is an invalid index.

Instead, what you should do if the items are not equal is assign them to the same index until you reach the correct item.

At this point, take the index, break the loop and start an other one in which you copy the items of the original array into the new one starting at ownedItem[index+1] into ownedItemCopy[index].

Community
  • 1
  • 1
EmileIB
  • 1
  • 1
0

To remove an element from an array, you should create a new array consisting of two parts of the previous array: before and after the item that being removed:

public static String[] removeItem(String[] arr, int item) {
    return Stream
            .concat(Arrays.stream(arr, 0, item),
                    Arrays.stream(arr, item + 1, arr.length))
            .toArray(String[]::new);
}
// test
public static void main(String[] args) {
    String[] arr = {"1", "2", "3", "4", "5"};
    String remove = "3";

    for (int i = 0; i < arr.length; i++) {
        if (arr[i].equals(remove)) {
            arr = removeItem(arr, i);
            break;
        }
    }

    System.out.println(Arrays.toString(arr)); // [1, 2, 4, 5]
}

You can simplify this code using ArrayList#removeIf method:

String[] arr = {"1", "2", "3", "4", "5"};
String remove = "3";

ArrayList<String> list = new ArrayList<>(List.of(arr));
// remove all elements that match the given predicate
list.removeIf(str -> str.equals(remove));
arr = list.toArray(String[]::new);

System.out.println(Arrays.toString(arr)); // [1, 2, 4, 5]

See also: Rotate an elements in an array between a specified range