1

I have this code that searches one object in an array and removes it. I'm having a problem with its position, since some other methods work with this array (and it gives me a NullPointerException every time). My method looks like this:

public void deleteHotel(String hotelName) {
    for (int i = 0; i < this.hoteis.length; i++) {
        if (this.hoteis[i].getName().equalsIgnoreCase(nomeHotel)) { //searches the array, looking for the object that has the inputted name
            this.hoteis[i] = null; //makes that object null
            if (this.hoteis.length > 1 && this.hoteis[this.hoteis.length - 1] != null) { //for arrays with lenghts bigger than 1 (since there's no problem with an array with one position)
                for (int x = i; x < this.hoteis.length; x++) {
                    this.hoteis[x] = this.hoteis[x + 1]; //makes that null position point to the next position that has an object, and then that position points to the object in the next position and so on
                }
                this.hoteis[this.hoteis.length - 1] = null; //since the last to positions will be the same, make that last one null
                Hotel[] hoteisTemp = new Hotel[this.hoteis.length - 1];
                for(int x = 0; x < this.hoteis.length - 1; x++){ //create a new array with one less position, and then copy the objects on the old array into the new array, then point the old array to the new array
                    hoteisTemp[x] = this.hoteis[x];
                }
                this.hoteis = hoteisTemp;
            }
            i = this.hoteis.length;
        }
    }

}

When I use other methods (for example, one that returns the implemented toString()s of each object) it gives me a NullPointerException. Can you guys identify the error in the code? Much appreciated...

  • possible duplicate of [How do I remove objects from an Array in java?](http://stackoverflow.com/questions/112503/how-do-i-remove-objects-from-an-array-in-java) – Jacob Krall Jul 17 '13 at 21:13
  • 1
    You do realize that this functionality comes _out of the box_ when using an `ArrayList`, right? All this code down to a simple: `list.remove("name")` – Colin M Jul 17 '13 at 21:14
  • I wouldn't be asking this question if I didn't, I would look for it. However, I want to try this with arrays. – user2486295 Jul 17 '13 at 21:26
  • Instead of re-arranging all the array, why not move the last object of the array to the position of deleted hotel? Then you can do your processing from start until length || nullpointer. This would require a variable to store the position of last object. Or it could be a simple counter. – Giannis Jul 17 '13 at 22:53

5 Answers5

1

The fundamental problem is that you're not allowing for where you removed the entry from the array.

Instead of

for(int x = 0; x < this.hoteis.length - 1; x++){

you want

for(int x = 0; x < this.hoteisTemp.length; x++){

(although that's a style choice)

and more significantly, instead of

hoteisTemp[x] = this.hoteis[x];

you want

int y = x < i ? x : x + 1;
hoteisTemp[x] = this.hoteis[y];

You also want to get rid of everywhere you're setting array elements to null, because if your copying logic works correctly, that's unnecessary.

For this use case, I would consider using one of the List implementations.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
1

Consider rewriting your code

List result = new ArrayList();
for (int i = 0; i < this.hoteis.length; i++) {
    if (!this.hoteis[i].getName().equalsIgnoreCase(nomeHotel)) {
        result.add(this.hoteis[i]);
    }
}
return result.toArray();
Anton
  • 5,831
  • 3
  • 35
  • 45
  • This is probably the simplest approach. Although perhaps best to actually *keep* the list, rather than going back to an array. – T.J. Crowder Jul 17 '13 at 21:25
1

I have tested your function and I see what you mean by it getting a nullpointerexception, this is due to the array not resizing the list - which is due to your conditional:
if (this.hoteis.length > 1 && this.hoteis[this.hoteis.length - 1] != null).
Simply removing this solved the issue, here is the working function:

public static void deleteHotel(String hotelName) {
    for (int i = 0; i < hotels.length; i++) {
        if (hotels[i].getName().equalsIgnoreCase(hotelName)) { //searches the array, looking for the object that has the inputted name
            hotels[i] = null; //makes that object null
            for (int x = i; x < hotels.length -1; x++) 
                hotels[x] = hotels[x + 1]; //makes that null position point to the next position that has an object, and then that position points to the object in the next position and so on

            Hotel[] hoteisTemp = new Hotel[hotels.length - 1];
            for(int x = 0; x < hotels.length - 1; x++) //create a new array with one less position, and then copy the objects on the old array into the new array, then point the old array to the new array
                hoteisTemp[x] = hotels[x];

            hotels = hoteisTemp;
            break;
        }
    }
}

Though please consider using a list of some sort when needing to use a list with a changing size.

Lee Fogg
  • 775
  • 6
  • 22
  • Ah, I see... thank you very much. I'm aware of Lists, but I was trying a different approach. You've all been very helpful, thank you. – user2486295 Jul 18 '13 at 03:33
0

The point where you're shifting the array elements towards the left

for (int x = i; x < this.hoteis.length; x++) {
    this.hoteis[x] = this.hoteis[x + 1];
}

The loop condition should be x < this.hoteis.length - 1 because at the last iteration when x = this.hoteis.length - 1 the index value this.hoteis[x + 1] would throw a NullPointerException.

Ravi K Thapliyal
  • 51,095
  • 9
  • 76
  • 89
0

Try using ArrayList it will simplify your code complexity.Here is the link to documentation. http://docs.oracle.com/javase/6/docs/api/java/util/ArrayList.html

rvd
  • 157
  • 1
  • 1
  • 9