# ArrayList.add() überschreibt vorhandene Einträge.



## KuShi (10. Jul 2012)

Hallo.
Ich versuche, Eine Textdatei zeilenweise einzulesen und diese Zeilen nach verarbeitung in einer ArrayList zu speichern. Hier mal die entsprechenden Code-Blöcke:

Einlesen aus Textdatei (Code-Schnipsel)

```
[List<IRWord>] sentence = new ArrayList<IRWord>();
					
					while ((line = readLine(in)) != null && !line.matches("</header>")) {
						while ((line = readLine(in)) != null && !line.matches("")) {
							sentence.add(processLine(line));
						}
					}
```

Hier die verwendete Methode processLine():

```
private IRWord processLine(String line) {
		IRWord word = new IRWord(line.split("\t"));

		return word;
	}
```

Jedes Mal, wenn ich ein neues Element (IRWord) in der ArrayList (sentence) speichere, werden die alten Elemente überschrieben. Ich weiß, dass man jedes Mal eine Neue Instanz der Elemente erstellen muss, bevor man diese in einer Liste speichert, aber ich dachte, das hätte ich in der Methode getan.
Könnt ihr mir bitte helfen, dieses Problem zu lösen, ich stehe irgendwie auf dem Schlauch.


----------



## timbeau (10. Jul 2012)

Ich verstehe die eckigen Klammern um die Liste nicht. Lass die mal weg


----------



## SlaterB (10. Jul 2012)

in welcher Weise zeigt sich 'überschrieben', was genau prüfst du?
ist die Anzahl der Elemente in der Liste ok?
alles dieselben Objekte können es anscheinend nicht sein,
aber wenn statische Variablen im Spiel sind, wirken vielleicht alle Objekte gleich?

Details und Infos, immer könnte es mehr sein,
idealerweise sowieso vollständige Testprogramme posten, auf Datei ist dabei vielleicht verzichten, 
einfach nur
> sentence.add(processLine("a"));
> sentence.add(processLine("b"));
> sentence.add(processLine("c"));
müsste doch auch eine Aussage haben


----------



## Korashen (10. Jul 2012)

Ich kann es leider gerade nicht nachstellen, aber splitte mal die Zeile 5 in zwei Zeilen:

```
IRWord irWord = processLine(line);
sentence.add(irWord);
```

Sollet das nicht helfen, lass die Zeilen so wie sie sind und debugge es mal:
Setze einen Breakpoint bei "sentence.add(irWord);" 
Merke dir mal die Objekt-ID von "irWord".
Ist es immer die gleiche ID, dann ist es immer das gleiche Objekt, heißt, dein Array mag zwar mehrere Einträge haben, die verweißen aber alle auf das gleiche Objekt.

Eigentlich solltest du eine neue Instantz des "word" Objekts in der Methode "processLine" bekommen, da die Variable ja nur im Methoden-Fokus besteht.

Sonst, versuch mal, die Methode "processLine" zu verkürzen auf:

```
private IRWord processLine(String line) {
        return new IRWord(line.split("\t"));
    }
```


----------



## jstei001 (10. Jul 2012)

Woher weißt du denn das die vorhanden Elemente überschrieben werden? Ist am Ende nur ein Element in dem Array? 

wegen den Eckigen Klammern müsste dir der Compiler eigentlich um die ohren fliegen. Nach welchen kriterien willst du die Zeilen denn verarbeiten?


----------



## nillehammer (10. Jul 2012)

Die add-Methode einer Liste *added* wirklich. D.h. das Element wird an das Ende der Liste geschrieben. Der Liste ist es auch egal, ob evtl. schon so ein Element vorhanden ist (egal, ob gleich oder sogar identisch).

Die Frage ist jetzt, wie der Effekt, von dem Du denkst es sei "Überschreiben" wirklich zustande kommt. Anhand der vorhandenen Informationen kann man da nur raten. Weist Du der Variablen 
	
	
	
	





```
sentence
```
 evtl. mehrfach einen Wert zu? Überschreibst *Du* also evtl. *die komplette Liste*?

Auch Dein verschachteltes while-Schleifenkonstrukt scheint mir fehleranfällig. Trenne die Bedingung für das Lesen aus dem Reader (
	
	
	
	





```
line!=null
```
) von den Bedingungen, die fachlich die weitere Verarbeitung steuern (
	
	
	
	





```
line.matches("</header>"))/line.matches(""))
```
)

Und noch zwei Performance-Tipps:



```
!line.matches("</header>"))
```
 Führt dazu, dass das Pattern, gegen das gematcht wird, jedes Mal neu kompiliert wird. Benutze besser

```
private static final Pattern HEADER_PATTERN = Pattern.compile("</header>");
...
HEADER_PATTERN.matcher(line).matches();
```



```
line.matches("")
```
, ein regex-Matching, um rauszufinden, ob ein String leer ist, ist Overkill. Da doch lieber 
	
	
	
	





```
isEmpty()
```
 oder 
	
	
	
	





```
length() == 0
```


----------



## KuShi (10. Jul 2012)

Danke für die flotten Antworten.



timbeau hat gesagt.:


> Ich verstehe die eckigen Klammern um die Liste nicht. Lass die mal weg



Die eckigen Klammern habe ich in das Script eingefügt, da dies nur ein Ausschnitt ist und im eigentlichen Programm steht an dieser Stelle nichts. Die Variable wird an anderer Stelle deklariert.



SlaterB hat gesagt.:


> in welcher Weise zeigt sich 'überschrieben', was genau prüfst du?
> ist die Anzahl der Elemente in der Liste ok?



In der Liste speichere ich Instanzen von einer Klasse IRWord, in der ich Attribute speichere, die ich aus der eingelesenen Textdatei-Zeile herauslese.
Geprüft werden die Instanzen durch eine einfache FOR-Schleife, die wie folgt aussieht (erweitertes Script)


```
sentence = new ArrayList<IRWord>();
					
					while ((line = readLine(in)) != null && !line.matches("</header>")) {
						while ((line = readLine(in)) != null && !line.matches("")) {
							irWord = processLine(line);
							sentence.add(irWord);
						}
						
						String sentenceString = "";
						for (int i=0; i<sentence.size(); i++) {
							sentenceString += sentence.get(i).getWord() + " ";
						}
						log(sentenceString, "file");
						
						header.add(sentence);
					}
				}
```



Korashen hat gesagt.:


> Ich kann es leider gerade nicht nachstellen, aber splitte mal die Zeile 5 in zwei Zeilen:
> 
> ```
> IRWord irWord = processLine(line);
> ...



Habe ich versucht (s.o.), funktioniert aber leider nicht.



Korashen hat gesagt.:


> Sollet das nicht helfen, lass die Zeilen so wie sie sind und debugge es mal:
> Setze einen Breakpoint bei "sentence.add(irWord);"
> Merke dir mal die Objekt-ID von "irWord".
> Ist es immer die gleiche ID, dann ist es immer das gleiche Objekt, heißt, dein Array mag zwar mehrere Einträge haben, die verweißen aber alle auf das gleiche Objekt.



Habe ich gemacht.
Die Variable irWord hat immer eine andere ID



Korashen hat gesagt.:


> Sonst, versuch mal, die Methode "processLine" zu verkürzen auf:
> 
> ```
> private IRWord processLine(String line) {
> ...



Funktioniert leider auch nicht 

Wenn ich das Script folgendermaßen schreibe:


```
String sentenceString = "";
					sentence = new ArrayList<IRWord>();
					
					while ((line = readLine(in)) != null && !line.matches("</header>")) {
						while ((line = readLine(in)) != null && !line.matches("")) {
							irWord = processLine(line);
							sentence.add(irWord);
							
							sentenceString += irWord.getWord() + " ";
						}
						log(sentenceString, "file");
						
						sentenceString = "";
						for (int i=0; i<sentence.size(); i++) {
							sentenceString += sentence.get(i).getWord() + " ";
						}
						log(sentenceString, "file");
						
						header.add(sentence);
					}
```

bekomme ich folgende Ausgabe:

wechselt zu Arsenal , Frankfurt holt Inui 
Inui Inui Inui Inui Inui Inui Inui 

D.h. die erste Zeile ist korrekt, die zweite nicht

--------- edit ---------



nillehammer hat gesagt.:


> Die add-Methode einer Liste *added* wirklich. D.h. das Element wird an das Ende der Liste geschrieben. Der Liste ist es auch egal, ob evtl. schon so ein Element vorhanden ist (egal, ob gleich oder sogar identisch).
> 
> Die Frage ist jetzt, wie der Effekt, von dem Du denkst es sei "Überschreiben" wirklich zustande kommt. Anhand der vorhandenen Informationen kann man da nur raten. Weist Du der Variablen
> 
> ...



Danke für die Tipps, ich werde diese beherzigen und meinen Code anpassen.
Interessant: Wenn ich folgenden Code einbaue:


```
irWord = processLine(line);
							sentence.add(irWord);
							
							sentenceString = "";
							for (int i=0; i<sentence.size(); i++) {
								sentenceString += sentence.get(i).getWord() + " ";
							}
							log(sentenceString, "file");
```

bekomme ich folgende Ausgabe:

wechselt 
zu zu 
Arsenal Arsenal Arsenal 
, , , , 
Frankfurt Frankfurt Frankfurt Frankfurt Frankfurt 
holt holt holt holt holt holt 
Inui Inui Inui Inui Inui Inui Inui 

werde mir das nochmal ansehen.


----------



## SlaterB (10. Jul 2012)

SlaterB hat gesagt.:


> aber wenn statische Variablen im Spiel sind, wirken vielleicht alle Objekte gleich?



du hast IRWord immer noch nicht gepostet,
es ist für die gesamte Welt und sogar Marsianer unklar, was getWord() liefert, 
kann auch von einem Zufallsgenerator abhängen ohne Code..


----------



## KuShi (10. Jul 2012)

SlaterB hat gesagt.:


> du hast IRWord immer noch nicht gepostet,
> es ist für die gesamte Welt und sogar Marsianer unklar, was getWord() liefert,
> kann auch von einem Zufallsgenerator abhängen ohne Code..



Sry. Hier die komplette Klasse:


```
public class IRWord {
	// global variables
	private static String word = null;
	private static String lemma = null;
	private static String wordForm = null;
	private static String pos = null;
	private static String label = null;
	private static int knot;
	private static String[] morphology = null;
	
	public IRWord(String[] wordArray) {
		word = wordArray[1];
		lemma = wordArray[2];
		wordForm = wordArray[3];
		pos = wordArray[4];
		morphology = wordArray[5].split("|");
		knot = Integer.parseInt(wordArray[6]);
		label = wordArray[7];
	}
	
	public String getWord() {
		return word;
	}
}
```


----------



## njans (10. Jul 2012)

Da alle Attribute statisch sind, wird dies immer überschrieben, wenn ein neues Objekt erzeugt wird. Der Sinn von static ist eben, dass es nur einmal existiert. Du speicherst so immer nur das neuste Objekt. 
Du musst die Attribute eben nicht static machen.


----------



## nillehammer (10. Jul 2012)

> Danke für die Tipps, ich werde diese beherzigen und meinen Code anpassen.


Und noch was: Die zweifach verschachtelte while-Schleife mit mehrfachem read ist schon schwer durchschaubar. Jetzt kommt darin noch eine for-Schleife! Über Ausgaben in merkwürdiger Reihenfolge brauchst Du Dich da nicht zu wundern. Refactor das unbedingt. Überlege, ob Du wirklich alle Schleifen brauchst. Falls ja, lager sie in eigene Methoden mit geeigneten Übergabeparametern/Return-Values aus. Damit bist Du dann auch in der Lage, jede Methode einzeln zu testen und findest den Fehler schneller.


----------



## KuShi (10. Jul 2012)

njans hat gesagt.:


> Da alle Attribute statisch sind, wird dies immer überschrieben, wenn ein neues Objekt erzeugt wird. Der Sinn von static ist eben, dass es nur einmal existiert. Du speicherst so immer nur das neuste Objekt.
> Du musst die Attribute eben nicht static machen.



Danke, das war die Lösung.

Danke euch allen für eure Hilfe.

--------- edit ---------



nillehammer hat gesagt.:


> Und noch was: Die zweifach verschachtelte while-Schleife mit mehrfachem read ist schon schwer durchschaubar. Jetzt kommt darin noch eine for-Schleife! Über Ausgaben in merkwürdiger Reihenfolge brauchst Du Dich da nicht zu wundern. Refactor das unbedingt. Überlege, ob Du wirklich alle Schleifen brauchst. Falls ja, lager sie in eigene Methoden mit geeigneten Übergabeparametern/Return-Values aus. Damit bist Du dann auch in der Lage, jede Methode einzeln zu testen und findest den Fehler schneller.



Diese For-Schleifen sind ja auch nur zum Test. Werde das noch umschreiben. Danke aber trotzdem für den Hinweis.


----------



## Korashen (10. Jul 2012)

Ah ja, mit dem Code zur IRWord-Klasse ist es leicht nach zu vollziehen, wieso du immer die gleichen Werte bekommst ;-)

Und die Moral von der Geschicht, vergiss die Klassen nicht 

Na, mehr oder weniger ;-)


----------

