Elemente in HashSet enthalten oder nicht

[Antonine]

Hallo,

ich möchte in einer Methode prüfen, ob ein bestimmtes Element in einem HashSet enthalten ist oder nicht. Wenn es noch nicht drin ist, soll es hinzugefügt werden. Analog soll dies mit zu löschenden Elementen passieren (wenn das zu löschende Element enthalten ist, dann soll es gelöscht werden).
Dachte, dass es nicht besonders kompliziert wäre, aber jedes Mal, wenn ich ein Element hinzufügen möchte, dass bereits im HashSet drin ist, wird es trotzdem hinzugefügt :/ Ebenso werden nicht enthaltene Elemente gelöscht.
Die Methoden sehen so aus:

//zum Hinzufügen
	public boolean addNode(Node node) { //Node enthält einen Wert vom Typ String
		Iterator<Node> i = nodes.iterator(); //nodes ist ein HashSet mit Elementen vom Typ Node
		boolean exists = false;
		while (i.hasNext()) {
			if(i.next().equals(node)) {
				exists = true;
			}
		}
		if(!exists) {
			nodes.add(node);
			JOptionPane.showMessageDialog(null, "Knoten erfolgreich hinzugefügt."); //wird immer angezeigt!!
			return true;
		} else {
			JOptionPane.showMessageDialog(null, "Knoten " + node.getValue().toString() + "ist \nbereits im Graph vorhanden.");
			return false;
		}
	}
	

//zum Löschen
public boolean removeNodeEdge(Node node) {		
		Iterator<Node> i = nodes.iterator();
		boolean exists = false;
		while (i.hasNext()) {
			if (i.next().equals(node)) {
				exists = true;
			}
		}
		if (!exists) {
			JOptionPane.showMessageDialog(null, "Der zu löschende Knoten \nexistiert nicht."); //wird nicht ausgegeben!!
			return false;			
		} else {
			nodes.remove(node);
			Iterator<Edge> it = edges.iterator();
		    while (it.hasNext()) {				
		      Edge edge = (Edge)it.next();
		      if (edge.getPredecessor() == node || edge.getSuccessor() == node) {
		        it.remove();
		      }
		    }	
		    JOptionPane.showMessageDialog(null, "Knoten erfolgreich gelöscht.");
			return true;
		}	    	
	}

Wäre super, wenn ihr mir helfen und den/die Fehler im Code verbessern könnt! Vielen Dank schon mal!

 

[andiv]

Wie sieht denn die Klasse Node aus? Insbesondere die equals- und die hashCode-Methode?

 

[Antonine]

Die equals-Methode sieht so aus:

public boolean equals(Node node) {
			boolean result = false;
			if (node.getValue() == this.value) {
				result = true;
			}
			return result;
		}

Eine hashCode-Methode habe ich nicht, könnte das das Problem sein...?

 

[AlexSpritze]

Eine hashCode-Methode habe ich nicht, könnte das das Problem sein...?

Ja! In der JavaDoc zur Methode

Object.equals(Object)

steht:

Note that it is generally necessary to override the hashCode method whenever this method is overridden, so as to maintain the general contract for the hashCode method, which states that equal objects must have equal hash codes.

Besonders auch, weil du deine

Node

in einem

HashSet

hälst, ist das Überschreiben dieser Methode wichtig

 

[xehpuk]

Nein, dass

hashCode()

nicht überschrieben ist, ist hier nicht das Problem!

Darauf kommt man schnell, wenn man sich anschaut, wie extrem umständlich der TE das Hinzufügen und Entfernen implementiert hat.

Ein java.util.Set (bitte genau betrachten) bringt bereits die Funktionalität mit, dass ein Element höchstens einmal in der Menge vorkommt. Die Verwendung eines Iterators ist also nicht nur unnötig, sondern ist auch wesentlich langsamer.

Im Prinzip sollten die beiden Methoden also so aussehen:

public boolean addNode(Node node) {
    boolean existed = nodes.add(node);
    if (existed) {
        JOptionPane.showMessageDialog(null, "Knoten erfolgreich hinzugefügt.");
    } else {
        JOptionPane.showMessageDialog(null, "Knoten " + node.getValue() + "ist \nbereits im Graph vorhanden.");
    }
    return existed;
}

public boolean removeNodeEdge(Node node) {
    boolean existed = nodes.remove(node);
    if (existed) {
        Iterator<Edge> it = edges.iterator();
        while (it.hasNext()) {              
            Edge edge = it.next();
            if (edge.getPredecessor().equals(node) || edge.getSuccessor().equals(node)) { // Nodes mit equals() vergleichen
                it.remove();
            }
        }  
        JOptionPane.showMessageDialog(null, "Knoten erfolgreich gelöscht.");
    } else {
        JOptionPane.showMessageDialog(null, "Der zu löschende Knoten \nexistiert nicht.");
    }
    return existed;
}

Damit das nun so funktioniert, müssen letztendlich

equals(Object)

und

hashCode()

überschrieben werden. Bei dir ist keine der beiden überschrieben!
Das wird deutlich, wenn du ein

@Override

vor dein

equals(Node)

setzt. Der Compiler wird meckern. Deswegen hat der Vergleich immer false ergeben.

Korrekt sieht die Methode dann so aus:

@Override
public boolean equals(Object obj) {
	if (this == obj)
		return true;
	if (this.getClass() != obj.getClass)
		return false;
	Node node = (Node) obj;
	return this.getValue().equals(node.getValue()); // Strings mit equals() vergleichen
}

Eine mögliche Implementation von

hashCode()

wäre:

@Override
public int hashCode() {
	return this.getValue().hashCode();
}

 

[Antonine]

Super, vielen Dank, so funktioniert's problemlos!

 

[bygones]

Die equals-Methode sieht so aus:

public boolean equals(Node node) {
			boolean result = false;
			if (node.getValue() == this.value) {
				result = true;
			}
			return result;
		}

sicher dass du den Value von node mit [c]==[/c] vergleichen willst und nicht auch per equals ?
ausserdem ist das nicht die equals von Object und somit nicht benutzbar