-1

I have created an array of objects (object representing a flight), and i'm trying to create a method to remove a specific object from that array, without changing it's length. I have written the following method :

public boolean removeFlight (Flight f) {

    for (int i = 0 ; i < _noOfFlights  ; i++) {
        if (_flightsSchedule[i].equals(f)) {
            _flightsSchedule[i] = _flightsSchedule[(i+1)];
            _noOfFlights--;
            return true;
        }
    }

    return false;
}

_noOfFlights represents the number of object currently in the array. For some reason it returns "false" when given an object that was added to the array.

Turing85
  • 18,217
  • 7
  • 33
  • 58
Shimi G
  • 9
  • 6
  • 4
    override equals in `Flight` – Pavneet_Singh Jan 16 '18 at 15:02
  • 2
    Your iteration variable is increasing while the conditional boundary is decreasing... Careful with that – OneCricketeer Jan 16 '18 at 15:03
  • 2
    You'll want to shift all the elements after the one to delete, not just the next one. – 001 Jan 16 '18 at 15:04
  • 2
    Your `(i+1)` will result in an out of bonds exception if the `Flight` is the last object in the array. – Murat Karagöz Jan 16 '18 at 15:04
  • Right. You need to set `_flightsSchedule[(i+1)]` to null to actually "remove it" – OneCricketeer Jan 16 '18 at 15:05
  • You are merely replacing a pointer inside the array.. as someone said above, try overriding the `equals` method and write a better management for the array positions. Alternatively, just set it to null and skip nulls when iterating – Repoker Jan 16 '18 at 15:05
  • This is also marked as the wrong dupe since he most likely did not override the equals/hashcode method. – Murat Karagöz Jan 16 '18 at 15:06
  • 2
    Some remarks on your code: - You should either write the `{` on the same line or a new line, but decide for one and stick with it. - Every variable- and fieldname (except for `final` primitives) should start with a lowercase letter. - Please take more care w.r.t. your indentation. – Turing85 Jan 16 '18 at 15:06
  • ovverride equals method – spandey Jan 16 '18 at 15:10
  • You're probably better off using an `ArrayList` and then walk over the elements using its `Iterator` by calling `iterator()` on it. The `Iterator` is able to remove an element from the list using `remove()`. – MC Emperor Jan 16 '18 at 15:24

1 Answers1

0

You need to be careful not to change the ground under your feet. You also don't want to return in the middle of the loop, otherwise you won't have moved all the elements properly.

You could do something like this:

public boolean removeFlight (Flight f) {
    boolean found = false;

    for (int i = 0 ; i < _flightsSchedule.length; i++) {
        if (f.equals(_flightsSchedule[i])) {
            found = true;
        } else if (found) {
            _flightsSchedule[i - 1] = _flightsSchedule[i];
        }
    }

    if (found) {
        _noOfFlights--;
        _flightsSchedule[_flightsSchedule.length - 1] = null;
    }

    return found;
}

Also, note that I've set the last element to null to avoid an inadvertent memory leak.

ᴇʟᴇvᴀтᴇ
  • 12,285
  • 4
  • 43
  • 66