-1

In the following function I have declared local variables allPeopel and itr(they are overriding global variables). If I comment out the local variables (between the Astrixes below), then a ConcurrentModificationError is thrown. However, if I use local variables instead of global variables then the code works fine. I don't understand why this is the case? There are many other functions in the class so I'm trying to use global variables for more efficient code.

   public void removeAPerson(){
        int id;
        Scanner sc = new Scanner(System.in);
        System.out.print("Enter ID of person to delete > ");
        id = sc.nextInt();
        sc.nextLine();
        System.out.println();

        /*************************************/
        ArrayList<Person> allPeople;
        allPeople = Person.getAllPeople();
        Iterator itr = allPeople.iterator();
        /*************************************/

        while(itr.hasNext()){
            Person obj = (Person) itr.next();
            int ID = obj.getID2();
            if(ID == id){
                itr.remove();
                break;
            }
        }
    }
Adam Arold
  • 29,285
  • 22
  • 112
  • 207
user1577173
  • 81
  • 3
  • 13
  • Your code is definitely not going to be more efficient with global variables. It is going to be more error-prone, though, and a maintenance nightmare. Finally, the only direction into which the performance may change with globals is **slower** since a heap var is slower to read than a stack/register var. – Marko Topolnik Nov 10 '12 at 13:40
  • @MarkoTopolnik Thanks. I'll stick to using local variables. I'd still like to understand why the error is thrown for global variables but not local ones though? – user1577173 Nov 10 '12 at 13:46
  • May be two methods trying to update same list same time? Without seeing your global implementation it is hard to guess. – kosa Nov 10 '12 at 13:48
  • A non-local iterator is almost every time a programming error. – Has QUIT--Anony-Mousse Nov 10 '12 at 14:00
  • @Nambari there is only a single thread so I don't think two methods can access access the list simultaneously. – user1577173 Nov 10 '12 at 14:20

2 Answers2

2

This is a sketch of what you probably have:

public class MyClass {
  List<Person> persons = new ArrayList<>();
  Iterator<Person> iter = strs.iterator();

  public void addPerson(Person p) {
    persons.add(p);
  }

  public void removePerson() {
    ... your posted code ...
  }


  public static void main(String... args) {
    MyClass c = new MyClass();
    c.addPerson(new Person());
    c.removePerson();
  }

What happens is that you instantiate the iterator only once, then add something to the list, then use the iterator. It never makes sense to reuse a global instance of iterator like that. Specifically, after you instantiate it, you must not change the list except through the iterator itself.

Using a global variable is one thing, but using a global instance of iterator is another. Both are wrong, but the latter is fatal.

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
0

Using local variables, each time a thread calls your method creates a new instance of these variables, that are not available to any other thread (unless you specifically allow for passing the references). So, no other thread can modify your objects.

Using global (I think you mean instance variables, attributes), all of the threads that have access to your object has access to those attributes of the object (let it be directly, let it be by running your object methods) and can modify them while your other thread is running the iteration. Since the iterator can detect when the Collection that is being iterated has been modified, when this happens it throws the exception (which means nothing else that "I was iterating all of the objects of the collection but these objects are no longer the same so this is too confusing and I fail".

SJuan76
  • 24,532
  • 6
  • 47
  • 87
  • By `global` I do mean `instance`. I'm not sure I follow you fully. I only have a single thread but I don't think that's what you mean. Different paths are available to me because of the switch, is this what you mean by different threads? The reason I'm confused is because, even though different functions can manipulate the ArrayList, only one at a time can have access. – user1577173 Nov 10 '12 at 14:25
  • Ok, I didn't think that your iterator would be an instance attribute too (as Marko said it just does not make sense) so I assumed that the modification of the collection had to be done at the some moment by some other code, running in a different thread. So I was assuming your code was correct (which it was not) which left only the option of a modification from other thread. – SJuan76 Nov 10 '12 at 14:47
  • thanks.it was my first time encountering an iterator. it makes sense to me now – user1577173 Nov 11 '12 at 03:23