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

[KuShi]

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]

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

 

[SlaterB]

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]

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]

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]

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]

Danke für die flotten Antworten.

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.

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);
					}
				}

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

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

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

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

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

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

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 ---------

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

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]

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]

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]

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]

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]

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 ---------

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]

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 ;-)