2010-02-26 41 views
14

¿Es esta una forma válida para encontrar y eliminar elemento de una LinkedList en Java usando una para cada bucle, es posible que pueda surgir inconsistencia:LinkedList: eliminar un objeto

for(ObjectType ob : obList) { 
    if(ob.getId() == id) { 
    obList.remove(ob); 
    break; 
    } 
} 

Respuesta

16

otros han mencionado el punto válido que normalmente esto no es la forma de remove un objeto de una colección. SIN EMBARGO, en este caso está bien desde break fuera del circuito una vez que remove.

Si desea seguir iterando después de remove, necesita utilizar un iterador. De lo contrario, obtendrá un ConcurrentModificationException o, en el caso más general, un comportamiento indefinido.

Así que sí, si break fuera de la foreach después remove, se le multa.


A los que está diciendo que esto se producirá un error porque no se puede modificar una colección en un foreach - esto es cierto sólo si desea mantener la iteración. Ese no es el caso aquí, por lo que este atajo está bien.

A ConcurrentModificationException es revisado y arrojado por el iterador. Aquí, después del remove (que califica como modificación simultánea), break fuera del ciclo. El iterador ni siquiera tiene la oportunidad de detectarlo.

Puede ser mejor si se añade un comentario en la break, por eso que es absolutamente necesario, etc, porque si este código se modificó más tarde para continuar la iteración después de un remove, fallará.

yo trataría este lenguaje similar a goto (o más bien, la etiqueta break/continue): puede parecer mal al principio, pero cuando se usa con prudencia, que lo convierte en un código más limpio.

+0

No he comprobado la especificación del idioma, pero ¿no es la sintaxis foreach simplemente la magia del compilador que usa un iterador detrás de las escenas? – Powerlord

+0

Sí, utiliza un iterador detrás de la escena y ese iterador está oculto para usted. – polygenelubricants

+3

Pero es muy probable que falle en la versión futura, cuando el desarrollador de nexte intenta agregar alguna funcionalidad sin "ver" esa trampa. Debes documentarlo claramente entonces. – whiskeysierra

4

Editar: De hecho, lo hará no fallar gracias al descanso. Vea la respuesta de Polygenelubricant para más detalles.

Sin embargo, esta es una forma peligrosa de hacerlo. Para iterar y modificar simultáneamente una colección en Java, debe usar el objeto "ListIterator" y usar los métodos "add()" y "remove()" del iterador, y no usar los que están en la colección.

puede comprobar el doc java para el "java.util.Iterator" y "clases" java.util.ListIterator

+0

"Iterator" no tiene un método "add()". Solo ListIterator tiene. Deberías señalar eso. – whiskeysierra

+1

@Zorglub, preste atención al 'break'. Este código no fallará – polygenelubricants

+3

Puede que no "falle", pero el código es, en cualquier sentido normal, incorrecto. –

1

intentar algo como esto:

Iterator<ObjectType> iter = obList.iterator(); 
while (iter.hasNext()) { 
    ObjectType ob = iter.next(); 
    if(ob.getId() == id) { 
    iter.remove(); 
    break; 
    } 
} 

Ese es uno de los últimos lugares en los que un iterador no puede ser reemplazado por un bucle foreach.

+0

No es necesario ser tan detallado si 'break' después de un' eliminar'. – polygenelubricants

+0

@polygenelubricants: oh, tienes razón. No me estaba dando cuenta de que la Excepción solo ocurriría en el siguiente ciclo. – Stroboskop

+2

Si esta es una lista vinculada, entonces Iterator.remove() es mucho más eficiente que List.remove (Object), ya que este último tiene que volver a buscar el objeto. Usaría el iterador y se eliminaría por principio. –

0

A CopyOnWriteArrayList podría ser lo que estás buscando. Cuando se realizan operaciones mutativas, se realiza una copia de la matriz subyacente. Esto permite la modificación de los elementos de la lista mientras se está dentro de un bucle for-each. Sin embargo, recuerde que esta no es una lista vinculada y puede ser bastante ineficiente.

import java.util.List; 
import java.util.concurrent.CopyOnWriteArrayList; 

public class Main { 

    public static void main(String[] args) { 
     List<String> myList = new CopyOnWriteArrayList<String>(); 

     myList.add("a"); 
     myList.add("b"); 
     myList.add("c"); 

     // Will print [a, b, c] 
     System.out.println(myList); 

     for (String element : myList) { 
      if (element.equals("a")) { 
       myList.remove(element); 
      } 
     } 

     // Will print [b, c] 
     System.out.println(myList); 
    } 

} 
6

Debe utilizar iterator.remove():

Quita de la colección subyacente el último elemento devuelto por el iterador (operación opcional). Este método solo se puede llamar una vez por llamada al siguiente. El comportamiento de un iterador es no especificado si la colección subyacente se modifica mientras que la iteración en curso en cualquier otra manera que por llamar a este método .

+0

"mientras la iteración está en progreso": esta es la distinción importante aquí. En su caso, una vez que modifica la colección, aborta la iteración. Es por eso que estamos teniendo esta discusión. Para el caso general, sin embargo, tienes toda la razón. – polygenelubricants

1

Para evitar una ConcurrentModifiationException , se puede hacer:

final Iterator<ObjectType> i = obList.iterator(); 
while (i.hasNext()) { 
    if (i.next().getId() == id) { 
     i.remove(); 
    } 
} 

o

for (int i = 0; i < obList.size(); i++) { 
    if (obList[i].getId() == id) { 
     obList.remove(i); 
    } 
} 

yo preferiría la primera. El manejo de índices es más propenso a errores y el iterador puede implementarse de manera eficiente. Y la primera sugerencia funciona con Iterable mientras que la segunda requiere una lista.

+0

Simplemente use el iterador en un ciclo for. –

+0

Eso dejaría vacía la última parte del ciclo. No me gusta eso. Un rato encaja perfectamente allí. – whiskeysierra

+0

Este código puede realizar múltiples eliminaciones. Es un comportamiento diferente al código original, que elimina como máximo un elemento. – polygenelubricants

7

Lo mejor es usar un iterador y utilizar su método de eliminación cuando se busca un objeto al iterar sobre una colección para eliminarlo. Esto se debe a

  1. La colección podría ser, por ejemplo, una lista enlazada (y en su caso se trata), cuyo método remove se entiende la búsqueda del objeto de nuevo, que la búsqueda podría tener O (n) la complejidad.
  2. No puede continuar la iteración después de la eliminación a menos que use el método de eliminación del iterador. En este momento está eliminando la primera incidencia; en el futuro, es posible que deba eliminar todas las coincidencias, en cuyo caso deberá volver a escribir el ciclo.

me recomiendan, en principio, renunciando a la mejorada y uso de algo como esto en su lugar:

for(Iterator<ObjectType> it=obList.iterator(); it.hasNext();) { 
    if(it.next().getId()==id) { 
     it.remove(); 
     break; 
     } 
    } 

De esa manera usted no está haciendo suposiciones acerca de la lista subyacente que podría cambiar en el futuro.


comparar el código para eliminar la última entrada llamada por el remove iterador (formateo de Sun):

private E remove(Entry<E> e) { 
    if (e == header) 
     throw new NoSuchElementException(); 

    E result = e.element; 
    e.previous.next = e.next; 
    e.next.previous = e.previous; 
    e.next = e.previous = null; 
    e.element = null; 
    size--; 
    modCount++; 
    return result; 
} 

en contra de lo quite (Objeto) debe hacer:

public boolean remove(Object o) { 
    if (o==null) { 
     for (Entry<E> e = header.next; e != header; e = e.next) { 
      if (e.element==null) { 
       remove(e); 
       return true; 
      } 
     } 
    } else { 
     for (Entry<E> e = header.next; e != header; e = e.next) { 
      if (o.equals(e.element)) { 
       remove(e); 
       return true; 
      } 
     } 
    } 
    return false; 
} 
+0

No respondió lo que le pregunté, pero es una buena idea. – Xolve

0

Lo anterior segundo ciclo debe cambiarse un poco

for (int i = 0; i < obList.size();) { 
    if (obList.get(i).getId() == id) { 
     obList.remove(i); 
     continue 
    } 
    ++i; 
} 

o

for (int i = obList.size() - 1; i >= 0; --i) { 
    if (obList.get(i).getId() == id) { 
     obList.remove(i); 
    } 
}