1 
String input from keyboard
Vector<String> myVector = new Vector<String>(someArray.length);   //assume Vector is populated
Iterator<String> itr = myVector.iterator();

for loop begins
    while(itr.hasNext() && itr.next().equals(input)){
       itr.remove();
    }

    ...

    while(itr.hasNext()    // is this the problem source?
     run more code  

for loop ends

when the current element equals to a string input, i want to remove that element, otherwise continue iterating. i keep getting concurrent exceptions here.

what else should i do? should i be moving my itr.next elsewhere?

QUESTION: i want the logic such that if the current Vector element equals to target, i want it removed from the Vector. how can i do that?

Improve this question
6
  • 3
    Is the Vector shared with other threads? Is there code modifying the Vector's contents within the loop that you have omitted? Commented Sep 17, 2012 at 19:56
  • Can you past full stack trace of exception? Commented Sep 17, 2012 at 19:57
  • 3
    Like a said in your previous question, your iterator isn't doing anything useful. You don't need it, take it out. Commented Sep 17, 2012 at 19:59
  • You are not populating the vector by using someArray.length, you are just setting it's capacity. Commented Sep 17, 2012 at 20:02
  • 4
    you'd be better off showing us the entirety of the code that is working with itr as it's likely that what you are omitting is the problem Commented Sep 17, 2012 at 20:04

5 Answers 5

Reset to default
2

I do not know why you are getting concurrent modification exceptions, because removing items through an iterator is legitimate: according to the documentation,

If the Vector is structurally modified at any time after the Iterator is created, in any way except through the Iterator's own remove or add methods, the Iterator will throw a ConcurrentModificationException.

To answer your question about removing from the vector all elements that are equal to target, the simplest solution is to use Vector's removeAll method.

myVector.removeAll(Collections.singletonList(input));
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

A ConcurrentModificationException can be thrown when iterating through a collection and not carefully removing elements from it.

I suggest you build a separate List to contain the elements to be removed and remove them all from the original Vector after the loop has finished executing.

Other suggestions:

You could also iterate over a copy of the list.

Use a foreach loop:

for (String value : myVector) {
  ...
}
Improve this answer

4 Comments

this isn't strictly true - it's perfectly legit to use an iterator and remove elements from it with (and only with) iterator.remove(). For example: for(Iterator it = list.iterator(); it.hasNext();) { if (someCondition) it.remove(); }
I agree. You can remove all elements in the separate list by simply calling originalList.removeAll( elementsToBeRemovedList ); As the guy above pointed out, you can use iterator.remove but beware that for certain subtypes, it will throw an unsupported operation exception. I prefer the method Dan mentioned as its just easier to understand...unless your list sizes are very very very large.
What do you mean by "carefully removing elements from it."?
See matt b's comment above and my answer starting from the second paragraph.
1

Have you initialized the contents of the vector? You are setting it's length in the constructor, but I can't see that you are actually adding strings to it, which will cause NullPointerException.

You probably want to initialize the Vector with: Arrays.asList(someArray)

When having a correct Vector, you don't need to have a while-loop for the iterator within a for-loop

Something like this should work:

String[] someArray = new String[]{ "A", "B", "C" };
Vector<String> myVector = new Vector<String>(Arrays.asList(someArray));
Iterator<String> itr = myVector.iterator();
while(itr.hasNext()){
   String myString = itr.next();
   if (myString.equals(input)) itr.remove();
}

EDIT The reason for the exception you got is most likely because you call the .next method incorrectly. The .next method should only be called once after each hasNext call, and .remove should only be called once after each .next call. Since you have omitted the details in your code, it's hard to pinpoint the problem exactly. But overall, there's no need for a for-loop. A while loop should be enough, but you should not have hasNext and next within an if-statement.

The correct way to iterate using an iterator is (in pseudo-code):

while (iterator has more items) {
    get the next item
    do something with the item (remove it if it should be removed, or handle it in another way)
}
Improve this answer

Comments

0

try wrapping your Vector like this:

Vector vector = Collections.synchronizedCollection(vector);

and a brief javadoc explanation:

Returns a synchronized (thread-safe) collection backed by the specified collection. In order to guarantee serial access, it is critical that all access to the backing collection is accomplished through the returned collection.

It is imperative that the user manually synchronize on the returned collection when iterating over it: 

Collection c =
 Collections.synchronizedCollection(myCollection);
      ...   synchronized(c) {
       Iterator i = c.iterator(); // Must be in the synchronized block
       while (i.hasNext())
          foo(i.next());   }
Improve this answer

5 Comments

This can be done if you are sure that the vector is not used in a multi-threaded environment (not shared by other threads), because synchronising it will make the vector locked for all other threads using it.
@dbf some synchronization must be done. There is no reason why not to do it in this case on the Vector itself. Other threads will have to wait till the operation is complete.
which isn't always desired that threads need to wait, specially if they are already in the execution code and get hold because they are not first in queue. I had this problem with a LPC and MBus controller, where communication was done via CAN. I can just say that it didn't always get the expected behaviour ..
@dbf What's better for the sake of the program: to wait till the removal operation is performed? or to allow threads to access semi-processed collection?
Funtik, the thread is locked in execution .. meaning ALL execution, if you have set a port listeners on that thread, which should be triggered if new data arrives, then it can't act, if it can't act but it's a vital piece of execution that should be done a response, then it won't until it can continue it's execution process... I'm just pointing out that a simple synchronisation is not a solid fix for all situations when dealt in a multi-threaded environment, that's all.
0

I'd just avoid the whole iterator. This should do what you want:

  while (myVector.remove(input)) {
    // this should get them all
  }
Improve this answer

3 Comments

It should work. But dasblinkenlight's example is better. It will get all of them that are equal to the input.
i tried yours it worked!! upvoting your answer.. hope you also upvote my question...i think if someone else posted this question with your answers, i would have solved it too!
Now I see that I downvoted your answer by mistake, and now I can't change it unless the answer is edited :/ This is actually a good solution.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.