Remove is not working in arraylist?

Question

I have project to do and i have to add people to the database and then remove them but when i try to remove a person from the arraylist it works but when i try to add more in i get index out of bounds exception?

public void removePerson(List<Person> CrecheList) {
    if (CrecheList.isEmpty()) {
        JOptionPane.showMessageDialog(null, "You need a Child or parent in the database", "Error", JOptionPane.INFORMATION_MESSAGE);
    } else {
        String pickid = JOptionPane.showInputDialog(null, "Please Enter an id");
        int id = Integer.parseInt(pickid);
        Iterator<Person> i = CrecheList.iterator();
        while (i.hasNext()) {
            Person p = i.next();
            if (p.getID() == id) {
                i.remove();
            } else {
                JOptionPane.showMessageDialog(null, "There is no such person in the database", "Child name", JOptionPane.INFORMATION_MESSAGE);
            }
        }
    }
}

and when i remove and try to add more into the arrylist i get index out of bounds?

Iterator is not guaranteed to support .remove(). Did it throw a "NotImplementedException"? — Paul Tomblin
– Paul Tomblin, Commented Nov 5, 2011 at 17:56
@Paul: I cite from the question: "when i try to remove a person from the arraylist it works". True, the question title is pretty bad and contradictory. — BalusC
– BalusC, Commented Nov 5, 2011 at 17:58

Mansoor Siddiqui · Accepted Answer · 2011-11-05 18:14:52Z

2

A completely alternative approach would be to implement the equals() method in your Person class so that it returns true if the ID fields are equal:

public class Person {
    int id;

    // Other fields/methods

    public boolean equals(Object o) {
        if (o instanceof Person) {
            Person p = (Person)o;
            if (this.id == p.getID()) return true;
        }
        return false;
    }
}

If you implement that, then you don't need to iterate over the elements -- you can simply call CrecheList.remove(p);.

edited Nov 5, 2011 at 18:14

answered Nov 5, 2011 at 18:04

Mansoor Siddiqui

22k11 gold badges51 silver badges69 bronze badges

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

Comments

Jason Buberel · Accepted Answer · 2011-11-05 17:58:05Z

1

Instead, pass a CopyOnWriteArrayList into your remove function, which allows for concurrent modifications, and then:

for ( Person p : CrecheList ) {
    if ( p.getID() == id ) {
      CrecheList.remove(p);
    }
}

answered Nov 5, 2011 at 17:58

Jason Buberel

5,0302 gold badges22 silver badges10 bronze badges

1 Comment

sean Over a year ago

What i was missing for i needed to update the array other wise it did not know it was removed.

Dave Newton · Accepted Answer · 2011-11-05 18:09:04Z

(This is not an answer.)

It may seem a bit overkill, but I'd totally break this code apart into its little functional bits to aid in testing.

public void removePerson(List<Person> CrecheList) {
    if (CrecheList.isEmpty()) {
        emptyListError();
        return;
    }

    int id = getId();
    if (!removePersonById(id)) {
        couldNotRemoveError();
    }
}

public boolean removePersonById(int id) {
    Iterator<Person> i = CrecheList.iterator();
    while (i.hasNext()) {
        Person p = i.next();
        if (p.getID() == id) {
            i.remove();
            return true;
        }
    }

    return false;
}

// Swing-specific stuff.

public void emptyListError() {
    JOptionPane.showMessageDialog(null, "You need a Child or parent in the database", "Error", JOptionPane.INFORMATION_MESSAGE);
}

public int getId() {
    String pickid = JOptionPane.showInputDialog(null, "Please Enter an id");
    return Integer.parseInt(pickid);
}

public void couldNotRemoveError() {
    JOptionPane.showMessageDialog(null, "There is no such person in the database", "Child name", JOptionPane.INFORMATION_MESSAGE);
}

Doing so allows you to test each functional component separately, and provides a simple mechanism for allowing different ways to remove people (for example, I always like having a CLI interface to most anything I'm doing).

stacker · Accepted Answer · 2011-11-05 19:56:08Z

0

Try to remove the person outside the iterating loop:

    Person p = null;

    while (i.hasNext()) { 
           p = i.next(); 
           if (p.getID() == id) { 
               break;
           }
           p = null;
    }            
    id ( p != null ) {
        CrecheList.remove( p );
    } 
    else { 
         JOptionPane.showMessageDialog(null, "There is no such person in the database", "Child name", JOptionPane.INFORMATION_MESSAGE); 
    }

edited Nov 5, 2011 at 19:56

answered Nov 5, 2011 at 17:59

stacker

69.1k29 gold badges143 silver badges214 bronze badges

3 Comments

Paul Tomblin Over a year ago

If the iterator supports .remove, then removing in the iterator loop is perfectly legal and works fine.

stacker Over a year ago

@Paul agreed, but he has problems with his code, so this is a suggestion to work around, we don't know the implementation of the list and its iterator.

Paul Tomblin Over a year ago

Unless he's overridden the Iterator to do something stupid, it should either correctly support .remove or it shoudl throw an exception.

Collectives™ on Stack Overflow

Remove is not working in arraylist?

4 Answers 4

Comments

1 Comment

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

1 Comment

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related