Don't repeat yourself: wie wiederholenden Code vermeiden?

Moin,

ich habe eine kleine Beispielmethode, die wie folgt aussieht:

public bool removeApplicationAtIndex(int index)
{
	// Look if the index makes sense to prevent Exceptions.
	// FIXME: shouldn't it be Count<index, since Count==index provides an OutOfBoundsException?
	// FIXME: shouldn't the index be checked to be larger than -1?
	if(this.applicationsDictionary.Count <= index)
	{
		// Doesn't make sense, stop trying.
		return false;
	}
	// Gather all keys in an array to determine which key represents this index.
	System.Collections.Generic.Dictionary<long,string>.KeyCollection kc = this.applicationsDictionary.Keys;
	int current = 0;
	foreach(long item in kc)
	{
		if(current == index)
		{
			// This key matches this index, so remove the object with this key.
			this.applicationsDictionary.Remove(item);
			return true;
		}
		current++;
	}
	// Index not found. Should never happen, but who knows.
	return false;		
}

Jetzt möchte ich dieselbe Funktionalität, also ein Mapping zwischen einem Array-Index und einem Dictionary-Key, auch für andere Dictionaries hinbekommen.

Mein erster Ansatz ist, die spezialisierten Methoden aufzuweichen und damit eine allgemeine Methode aufzurufen.

public bool removeApplicationAtIndex(int index)
{
    return removeValueInDictAtIndex(this.applicationsDictionary, index);
}
public bool removeValueInDictAtIndex(System.Collections.Generic.Dictionary<object,object> dict, int index)
{
...
    if(dict.Count <= index)
...
    System.Collections.Generic.Dictionary<object,object>.KeyCollection kc = dict.Keys;
...
    foreach(var item in kc)
...
            dict.Remove(item);
...
}

Nur leider habe ich unterschiedliche Dictionaries.
Eines arbeitet mit <string,string>, eines mit <long,string>, eines mit <string,long> und so weiter. Deshalb ist dieser Ansatz nicht praktikabel.

Wie kann ich es realisieren, dass ich obiges Gewirr (das vermutlich auch ein paar Fehler enthält, siehe //FIXME: ^^) nur einmalig tippen muss und trotzdem unterschiedliche Dictionaries abgearbeitet werden können?

Benutze doch Generics dafür

public bool RemoveKey<TKey, TValue>(IDictionary<TKey, TValue> dictionary, int index)

Macht es in einem Dictionary Sinn, anhand dem Index auf ein Item zu schließen?
Man weiß doch eigentlich nicht in welcher Reihenfolge die Keys da drin stehen.
Vielleicht hast du ja diesbezüglich noch einen Denkfehler in deinem Ablauf.

pdelvo
Danke für den Tipp mit den Generics. Das hatte ich bereits probiert, doch da muss irgendwo ein Fehler in meiner Vorgehensweise gewesen sein.

chilic
Guter Punkt.

In meinem Fall habe ich ein Dictionary gespeichert.
Die Werte dieses Dictionaries möchte ich in einer ListBox anzeigen.
Da ich faul bin, lasse ich mir einfach aus den Werten eine BindingList erstellen und verheirate diese als Datenquelle mit der ListBox.

public System.ComponentModel.BindingList<string> getApplicationNames()
{
	// The simplest way is to create a new BindingList object and paste all dictionary items to it.
	System.ComponentModel.BindingList<string> names = new System.ComponentModel.BindingList<string>();
	foreach(var item in applicationsDictionary)
	{
		names.Add(item.Value);
	}
	return names;
}
...
private void initializeApplicationsList()
{
	listOfApplications = applicationAndDevicesListDatasource.getApplicationNames();
	listOfApplications.AllowNew = true;
	listOfApplications.AllowRemove = true;
	listOfApplications.AllowEdit = false;
	this.applicationListBox.DataSource = listOfApplications;
}

Wenn jetzt der Benutzer in der ListBox etwas auswählt und den 'Löschen'-Button anklickt, dann soll das gewählte Element naheliegenderweise gelöscht werden - sowohl aus der ListBox als auch aus dem Dictionary.

Ich hoffe einfach, dass in der durch die Iteration erstellten BindingList die Stringobjekte in der Reihenfolge angelegt wurden, wie sie auch im Dictionary anzutreffen sind. Nach mehreren Tests war das auch der Fall.
Weiterhin hoffe ich, dass das Dictionary während der Laufzeit seine interne Anordnung nicht umstrukturiert. Hat es bis jetzt nicht getan und dieses Verhalten steht auch nirgendwo dokumentiert.

Insofern denke ich, dass dieser Ablauf keinerlei Denkfehler enthält.
Wenn dir dennoch einer auffällt bin ich für jeden Hinweis dankbar. 😃

Ich hoffe einfach, dass in der durch die Iteration erstellten BindingList die Stringobjekte in der Reihenfolge angelegt wurden, wie sie auch im Dictionary anzutreffen sind. Nach mehreren Tests war das auch der Fall.
Weiterhin hoffe ich, dass das Dictionary während der Laufzeit seine interne Anordnung nicht umstrukturiert. Hat es bis jetzt nicht getan und dieses Verhalten steht auch nirgendwo dokumentiert.

Nun ja - da es nicht dokumentiert ist, kann sich natürlich die interne Implementierung ändern. Auch wenn eine Änderung des Verhaltens nicht sehr wahrscheinlich ist, sollte aber doch besser eine geeignete Datenstruktur verwendet werden. Wie unschön eine Index in Deiner Lösung zu finden ist, siehst Du ja.
Also nimm z. B. das OrderedDictionary oder eine SortedList.

OrderedDictionary sieht nett aus.
Ich schau mal, was ich damit so hinbekomme.
Vielen Dank!

So sieht es jetzt umgebaut auf OrderedDictionary aus:

public bool removeApplicationAtIndex(int index)
{
	return removeItemFromDictionaryAtIndex(this.applicationsDictionary, index);
}
		
private bool removeItemFromDictionaryAtIndex(System.Collections.Specialized.OrderedDictionary dictToDeleteFrom, int index)
{
	// Look if the index makes sense to prevent Exceptions.
	if(dictToDeleteFrom.Count < index || index < 0)
	{
		// Doesn't make sense, stop trying.
		return false;
	}
	
	try
	{
		dictToDeleteFrom.RemoveAt(index);	
	}
	catch
	{
		// Should only occour if dictionary is readonly since out-of-bounds is handled above
		return false;
	}
	return true;
}

Ich bitte um Nachsicht, da ich im normalen Tagesgeschäft maximal 9 Collections zur Verfügung habe und mir spezialisierte Collections weitestgehend unbekannt sind. 😉

Wenn dir dennoch einer auffällt bin ich für jeden Hinweis dankbar. 😃

Wenn die Reihenfolge stimmt, sollte es ja passen.

Aber was anderes, wenn du das zu löschende Item kennst (und du kennst das ja wenn du die Items in einer ListBox hast), hast du dann nicht auch den Key davon und kannst mit dem direkt löschen?
Oder du schaust direkt hinter welchem Key dein Objekt steckt und hast dann dein Kriterium zum löschen. Dazu musst du ja nicht erst alle Keys raussuchen und dann zählen.

Ich bin jemand der nicht so gerne auf irgendwas hofft das sich evtl. mal ändert 😉

Hallo,

Also ich würde da auch nicht auf den Index vertrauen.

    System.ComponentModel.BindingList<string> names = new System.ComponentModel.BindingList<string>();
    private void Form1_Load(object sender, EventArgs e)
    {
      for (int i = 0; i < 20; i++)
      {
        names.Add("Name " + i.ToString());
      }
      listBox1.DataSource = names;
    }

    private void button1_Click(object sender, EventArgs e)
    {
      names.Remove((string)listBox1.SelectedItem);
    }

sg