Während einer Codeüberprüfung mit Sonar wurde der folgende Code als schlecht erkannt:
%Vor%Sonar beschwert sich über das removeAll on, das von der Sammlung selbst aufgerufen wird.
Ich stimme zu, dass es hässlich ist, aber kann das Bugs verursachen?
NB: Dies ist nicht mein Code, ich überprüfe es.
Das Problem besteht darin, ob eine ConcurrentModificationException - oder Listenbeschädigung oder Endlosschleife oder ein Fehler beim Entfernen von Einträgen oder ähnlichem auftreten kann.
ArrayList , speziell in Oracle JDK8, scheint so geschrieben zu sein, dass diese Probleme nicht auftreten.
Bedeutet das, dass dieser Code dann in Ordnung ist?
Nein, es ist nicht in Ordnung.
Dieser Code:
Verweist auf die Implementierung der removeAll der Liste, um intelligent genug zu sein, um einen sehr seltsamen Anwendungsfall zu behandeln
Ist unnötig komplex zu lesen und zu verstehen, wodurch Wartungsprobleme entstehen
Er macht unnötige Arbeit und braucht daher länger, um seine Arbeit zu erledigen, als nötig (nicht, dass dies wahrscheinlich eine große Sache ist)
Sie haben dies im Kontext der Code-Überprüfung gesagt. Ich würde es markieren und mit dem Autor darüber sprechen, warum sie es verwendet haben, und erklären, warum ops.clear(); oder ops = new ArrayList<String>(); (abhängig vom Kontext) mit Sicherheit eine bessere Wahl wäre, von einer Zuverlässigkeit, Wartung und (sehr gering) Leistungsperspektive.
Ja, das wird Bugs einführen. Der Standardwert removeAll funktioniert nach dem Prinzip eines Iterator . Wenn Sie die Sammlung ändern, ohne den Iterator zu verwenden, erhalten Sie ConcurrentModificationException . Ob es diese Ausnahme gibt oder nicht, hängt vom internen Design des von Ihnen verwendeten Collection ab und kann nicht verwendet werden.
Obwohl die aktuelle Version kein iterator() verwendet, ist dies nicht dokumentiert, und Oracle kann dies ohne vorherige Ankündigung ändern.
Um eine Sammlung zu löschen, können Sie .clear() verwenden.
Dies könnte absolut Fehler einführen. Abhängig von der Implementierung einer Sammlung könnte dies insbesondere ConcurrentModificationException .
Um zu verstehen, wie dies passieren könnte, betrachten Sie diese Pseudo-Implementierung:
%Vor% Sobald wir einen Eintrag aus dieser Liste entfernen, wird der Iterator von collection ungültig. Die äußere for-each-Schleife kann nicht fortfahren und verursacht eine Ausnahme.
Natürlich ist die tatsächliche Implementierung anders und verhindert diese Art von Bugs. Es gibt jedoch keine Garantie in der Dokumentation, dass alle Implementierungen auf diese Weise "defensiv" sein müssen. daher die Warnung.
Mehr als nur Fehler, es sieht nach einer falschen Verwendung von removeAll () in Arrays aus. Java doc sagt "removeAll ()" entfernt alle Elemente dieser Sammlung, die auch in der angegebenen Sammlung enthalten sind. Nachdem dieser Aufruf zurückgegeben wurde, enthält diese Sammlung keine Elemente mehr mit der angegebenen Sammlung.
Wenn also alle Elemente gelöscht werden sollen, ist clear () die Methode, die aufgerufen wird, anstatt removeAll. Letzteres sollte verwendet werden, wenn die Elemente entfernt werden sollen, die auch in der angegebenen Sammlung enthalten sind.
Hoffe, das hilft.
Tatsächlich hängt es von der Version von jdk ab, die Sie verwenden. In jdk6 erben die meisten Auflistungsklassen die removeAll-Methode von der AbstractCollection-Klasse:
%Vor% In einem Szenario wird dies beispielsweise zu einem CME ( ConcurrentModificationException ) führen:
In jdk8 haben die meisten Sammlungen jedoch ihre eigene removeAll-Implementierung, z. B. ArrayList. Die ArrayList-Klasse hat ihre eigene removeAll-Methode, die eine private Methode batchRemove:
aufruft %Vor%Das Verwenden der for-Schleife iteriert über das zugrunde liegende Array und ändert die Variable modCount nur einmal nach dem Ende dieser Methode während der Iteration der Sublist (durch c.contains ( elementData [r]) ), der modCount ändert sich nicht, daher wird kein CME ausgelöst.
Tags und Links java collections