# "Doppelte" Einträge einer Liste entfernen



## qwrsrg43 (5. Okt 2011)

Moin moin,

ich bekomme aus der Datenbank eine Liste von ValueObjekten. Diese VO's sehen sich alle sehr ähnlich, sie unterscheiden sich vllt in 1 - 2 Werten.

Ums besser zu veranschaulichen: 
id | kdNr | Bezeichnung | trNr
--+-----+-------------+----------
1 | 23    | AE              | 123456
2 | 23    | BE              | 123456
3 | 24    | TE              | 123456
4 | 25    | TE              | 123456
...

Ich möchte diese Liste jetzt "bereinigen". Da für meinen Fall die Bezeichnung völlig unwichtig ist möchte ich nur eines der Objekte mit der kdNr 23 erhalten. Welches spielt keine Rolle.
Ich bekomme bei meinem Versuch eine java.util.ConcurrentModificationException
Hier mein Versuch:


```
final List<TrVO> publish = new ArrayList<TrVO>();
final List<TrVO> entities = traeger.getAllTr(string, string, string, string, string);
            for (final TrVO entity : entities) {
                if (!publish.isEmpty()) {
                    for (final TrVO p : publish) {
                        if (!p.getKundenNummer().equals(entity.getKundenNummer())) {
                            publish.add(entity);
                        }
                    }
                } else {
                    publish.add(entity);
                }
            }
```

Hat vllt jmd eine funktionierende Idee ?

Danke im voraus


----------



## The_S (5. Okt 2011)

Wenn du nicht auf die Reihenfolge angwiesen bist, verwende ein Set anstatt einer Liste.


----------



## darekkay (5. Okt 2011)

Erstmal fällt deine getAllTr-Methode auf - wenn du den gleichen String übergibst, dann solltest du es an ein getAllTr(String string){...} anpassen - denn die Parameter solltest du innerhalb der Methode nicht verändern.

```
for (final TrVO p : publish) {
                        if (!p.getKundenNummer().equals(entity.getKundenNummer())) {
                            publish.add(entity);
                        }
                    }
```

Das kann natürlich nicht funktionieren.Beispiel:

23 hinzugefügt
23 prüfen - wird nicht hinzugefügt, weil schon drin
24 hinzugefügt
24 nochmal hinzugefügt (als erstes wird es ja schließlich mit der ersten 23 verglichen - und es es ungleich ist, wird es hinzugefügt).



The_S hat gesagt.:


> Wenn du nicht auf die Reihenfolge angwiesen bist, verwende ein Set anstatt einer Liste.



Das wird nicht funktionieren, da

23|TE|123
23|TE|321

nicht als gleiche Objekte angesehen werden, was aber hier explizit gewünscht ist.

*Lösungsvorschlag*: sollten die Kundennummer sortiert sein, musst du das Element, was du hinzufügen möchtest, mit dem letzten Element deiner neuen Liste vergleichen.


----------



## qwrsrg43 (5. Okt 2011)

Die getAllTr-Methode funktioniert richtig, ich habe nur die Paramter durch "string" ersetzt, weil hinter den Paramter restrictions stecken die für mein Problem nicht von belangen sind


----------



## qwrsrg43 (5. Okt 2011)

darekkay hat gesagt.:


> *Lösungsvorschlag*: sollten die Kundennummer sortiert sein, musst du das Element, was du hinzufügen möchtest, mit dem letzten Element deiner neuen Liste vergleichen.



Die Kundennummern sind Leider nicht sortiert.

Trotzdem danke


----------



## The_S (5. Okt 2011)

qwrsrg43 hat gesagt.:


> Die Kundennummern sind Leider nicht sortiert.
> 
> Trotzdem danke



Dann verwende ein Set.



darekkay hat gesagt.:


> Das wird nicht funktionieren, da
> 
> 23|TE|123
> 23|TE|321
> ...



Sind doch eh in TrVO-Objekten gekapselt. equals und hashcode entsprechend überschreiben und gut is.


----------



## SlaterB (5. Okt 2011)

das add() muss nicht in der inneren Schleife mit dem equals stehen, bzw. du hast es im Moment eh zweimal, 
strukturiere um:

```
for alle Elemente {
   boolean add = true;
   wenn leer dann add schon ok als true
   sonst innere Schleife: falls Id zu finden add auf false setzen

   nun ganz am Ende aller Prüfungen wenn add == true, dann einfügen
}
```

zu deinem bisherigen:
generell musst du bedenken: wenn eine Kundennummer nicht übereinstimmt ist das noch nicht ein Grund einzufügen, 
selbst wenn es ohne Exception ginge:
wenn schon 10 Elemente in der Liste sind wird mindestens 9x die Id nicht übereinstimmen,
mindestens 9x eingefügt egal ob Id schon drin oder nicht,
nein, erst die ganze Liste anschauen, dann erst ist die Information zur Entscheidung da


----------



## darekkay (5. Okt 2011)

qwrsrg43 hat gesagt.:


> Die getAllTr-Methode funktioniert richtig, ich habe nur die Paramter durch "string" ersetzt, weil hinter den Paramter restrictions stecken die für mein Problem nicht von belangen sind



Ach ok, dann macht's natürlich Sinn.



qwrsrg43 hat gesagt.:


> Die Kundennummern sind Leider nicht sortiert.



Wenn du keinen Wert auf die Effizienz legst und die Datenmenge überschaubar ist, dann schreibe eine solche Methode:


```
boolean exists(String kdNr){
  for(TrVO p : list)
    if(p.getKundenNummer().equals(kdNr))
      return true;
  return false;
}
```

(bzw. list als Parameter übergeben, falls nicht global)

Damit musst du schon mal nicht die ganze Liste durchgehen, da die Schleife beim ersten Treffer stoppt.
Nun müsstest du eig. nur noch deine erste Liste durchgehen, für jedes Element die exists-Methode aufrufen, und bei false das Element hinzufügen.



The_S hat gesagt.:


> Sind doch eh in TrVO-Objekten gekapselt. equals und hashcode entsprechend überschreiben und gut is.



Stimmt natürlich, daran habe ich nicht gedacht.


----------



## oopexpert (5. Okt 2011)

Die ConcurrentModificationException erhälst Du, weil Du über die Liste, die Du veränderst (publish) iterierst und gleichzeitig Elemente hinzufügst.

Grundsätzlich würde ich für so eine Bereinigungsaktion folgendermaßen vorgehen:
1. TrVO wrappen
2. Equals-Methode des Wrappers definieren
3. Ein Set als Ergebnismenge verwenden (z.B. HashSet)


----------



## The_S (5. Okt 2011)

oopexpert hat gesagt.:


> Die ConcurrentModificationException erhälst Du, weil Du über die Liste, die Du veränderst (publish) iterierst und gleichzeitig Elemente hinzufügst.
> 
> Grundsätzlich würde ich für so eine Bereinigungsaktion folgendermaßen vorgehen:
> 1. TrVO wrappen
> ...



Danke. Hab schon gedacht Sets und vorgefertigte Strukturen sind außer Mode.


----------



## oopexpert (5. Okt 2011)

> Sind doch eh in TrVO-Objekten gekapselt. equals und hashcode entsprechend überschreiben und gut is


Ich würde das einen Wrapper erledigen lassen, damit die Semantik des TrVOs nicht geändert wird. Die Frage ist, ob ein TrVO so eine Konsistenzbedingung ins sich "tragen" sollte. Man hat ja nur eine equals-Methode. Aber da muss man sich anschauen, was mit einem TrVO an anderen Stellen passiert. Weil ich das nicht weiss, die Idee mit dem Wrapper.


----------



## qwrsrg43 (5. Okt 2011)

darekkay hat gesagt.:


> Wenn du keinen Wert auf die Effizienz legst und die Datenmenge überschaubar ist, dann schreibe eine solche Methode:
> 
> 
> ```
> ...



Danke!! Auf dem Weg hats geklappt. Hier mein Code

```
final List<TraegerreferenzVO> publish = new ArrayList<TraegerreferenzVO>();
final List<TraegerreferenzVO> entities = traeger.getAllTraegerReferenzen("string", "string", "string", "string", "string");

            for (final TrVO entity : entities) {
                final boolean trigger = exists(entity.getKundenNummer(), publish);
                if (!trigger) {
                    publish.add(entity);
                }
            }
```

Hier die exists-Methode:

```
private boolean exists(final Long kundenNummer, final List<TrVO> entities) {
        if (!entities.isEmpty()) {
            for (final TrVO tr : entities) {
                if (!tr.getKundenNummer().equals(kundenNummer)) {
                    return false;
                }
            }
            return true;
        }
        return false;
 }
```

Vielen Dank für Eure Hilfe !


----------



## SlaterB (5. Okt 2011)

der Code ist noch falsch, die exists-Methode hat darekkay dir doch perfekt vorgegeben, deine funktioniert im allgemeinen nicht,

wie bereits geschrieben für deinen alten Code:
wenn die erste Id in der Liste nicht mit der Such-Id übereinstimmt, hat das für sich keine endgültige Entscheidung,
dann kannst du nicht schon exists = false zurückgeben, es kann doch sein dass die zweite Id in der Liste dann übereinstimmt

boolean-Variablen nichtssagen trigger zu benennen ist übrigens schlecht,
das beschreibt überhaupt nicht was denn nun ein Wert true oder false bedeutet, man muss alles immer wieder nachschlagen, 
z.B. auf die exists-Methode schauen, die gut benannt ist,
nenne die boolean-Variable z.B. auch exists, nicht originell, aber hilfreicher als trigger, oder ohne Variable gleich die Methode im if abfragen

if (!exists) {
                    publish.add(entity);
}
liest sich dann jedenfalls wie ein Satz "wenn nicht vorhanden, dann einfügen"


----------



## The_S (5. Okt 2011)

oopexpert hat gesagt.:


> Ich würde das einen Wrapper erledigen lassen, damit die Semantik des TrVOs nicht geändert wird. Die Frage ist, ob ein TrVO so eine Konsistenzbedingung ins sich "tragen" sollte. Man hat ja nur eine equals-Methode. Aber da muss man sich anschauen, was mit einem TrVO an anderen Stellen passiert. Weil ich das nicht weiss, die Idee mit dem Wrapper.



Jo, kommt auf den Kontext an. Wrapper ist halt zusätzlicher Overhead. Wenns ne eigene Klasse ist, die sowieso nur er verwendet, kann er auch in der Klasse direkt die Methoden entsprechend implementieren. Aber das Thema ist ja schon gelöst, heutzutage macht man sich lieber Arbeit zweimal  .

Gibts eigentlich nen Standardset, das Objekte aufgrund eines externen Algorithmus, bspw. Comparator, vergleicht?


----------



## freez (5. Okt 2011)

Wie wäre es mit einer HashMap mit der kdNr als Key? Da muss die Klasse gar nicht angefasst werden, keine Vergleiche, einfach nur [c]map.put(p.getKundenNummer(), p);[/c].


----------

