Liste Filtern mit bestimmten Werten

[shao123]

var liste = new List<Person>();

            liste.Add(new Person
            {
                Vorname = "Frank",
                Nachname = "Müller",
                Geburtsdatum = "02.12.1980",
                Wert = "1"
            });

            liste.Add(new Person
            {
                Vorname = "Frank",
                Nachname = "Müller",
                Geburtsdatum = "02.12.1980",
                Wert = "2"
            });

            liste.Add(new Person
            {
                Vorname = "Thomas",
                Nachname = "Herbst",
                Geburtsdatum = "02.12.1980",
                Wert = "3"
            });

            liste.Add(new Person
            {
                Vorname = "Frank",
                Nachname = "Müller",
                Geburtsdatum = "02.12.1980",
                Wert = "4"
            });

             var listDis = liste.Select(a => new { a.Vorname, a.Nachname, a.Geburtsdatum }).Distinct().ToList();
        }
    }

    public class Person
    {
        public string Vorname;
        public string Nachname;
        public string Geburtsdatum;
        public string Wert;
    }

Hallo Leute,

ich hänge an was doofem fest. Ich habe eine List von der Klasse Person. Diese hat 4 Werte wie ihr in meinem Code sehen könnt. Ich möchte ein Distinct über die Liste. Allerdings nur über Vorname, Nachname und Geburtsdatum.

Meine Lösung führt das zwar aus. Aber schneidet den 4 Wert ab. Dieser wird aber trotzdem benötigt

Hat wer einen Lösungsidee?

Danke und Gruß

[arlegermi]

Naja, der Standard-Wurf wäre ein DistinctBy:

public static IEnumerable<TSource> DistinctBy<TSource, TKey>(this IEnumerable<TSource> source, Func<TSource, TKey> keySelector)
{
	if(source == null)
	{
		throw new ArgumentNullException(nameof(source));
	}
	
	var distinctSet = new HashSet<TKey>();
	foreach(var element in source)
	{
		if(distinctSet.Add(keySelector(element)))
		{
			yield return element;
		}
	}
}

Das sähe für deinen Fall dann vom Aufrufen so aus:

var distinct = liste.DistinctBy(p => new {p.Vorname, p.Nachname, p.Geburtsdatum});

Es gibt einige Libraries, die sowas enthalten, z.B. MoreLINQ. Musst du wissen, ob du für so eine Sache gleich 'ne ganze Library reinholen willst oder nicht einfach eine eigene Extension-Methode schreibst.

Wenn man das ganz extrem sauber machen möchte, teilt man das noch in zwei Methoden auf:

public static IEnumerable<TSource> DistinctBy<TSource, TKey>(this IEnumerable<TSource> source, Func<TSource, TKey> keySelector)
{
	if(source == null)
	{
		throw new ArgumentNullException(nameof(source));
	}
	
	return DistinctByImpl(source, keySelector);
}

private static IEnumerable<TSource> DistinctByImpl<TSource, TKey>(IEnumerable<TSource> source, Func<TSource, TKey> keySelector)
{
	var distinctSet = new HashSet<TKey>();
	foreach (var element in source)
	{
		if (distinctSet.Add(keySelector(element)))
		{
			yield return element;
		}
	}
}

Hat dann den Vorteil, dass die ArgumentNullException nicht erst geschmissen wird, wenn man wirklich über die Menge iteriert, sondern direkt an der Stelle, wo man die Methode aufruft (macht das Debuggen einfacher).

Bearbeitet 12. Juni 2018 von arlegermi

[Whiz-zarD]

In Distinct kann man auch einen Comparer mitgeben:

var listDis = liste.Distinct(new PersonComparer()).ToList();

public class PersonComparer : IEqualityComparer<Person>
{
    public bool Equals(Person x, Person y)
    {
        if (x == null || y == null)
            return false;

         return x.Vorname == y.Vorname 
             && x.Nachname == y.Nachname 
             && x.Geburtsdatum == y.Geburtsdatum;
     }

    public int GetHashCode(Person obj)
    {
        return obj.Vorname.GetHashCode()
               ^ obj.Nachname.GetHashCode()
               ^ obj.Geburtsdatum.GetHashCode()
               ^ obj.Wert.GetHashCode();
     }
}

 

[OMem]

Ich habe keine Ahnung von .NET, also nehme ich an das das von Whiz-zarD richtig ist;

aber eine grundsätzlich unschöne sache ist das deine neue liste nicht aus deiner extra erstellten Klasse Person besteht sondern aus einem Array? (oder was new {String,String,String} auch immer erzeugt) ist. Du kannst also alle methoden etc die du für deine PersonenKlasse erstellst nicht darauf anwenden. Das heißt selbst wenn du new{Name,Vorname,GebDat,Wert} erstellen könntest wäre es oft geschickter trotzdem eine liste<Person> zu erstellen.

(Falls .NET irgendeine Magie kann die ich nicht kenne und er ein new{} trotzdem wie ein Person behandeln kann dann denn Kommentar ignorieren)

[Whiz-zarD]

vor 2 Minuten schrieb OMem:

Ich habe keine Ahnung von .NET, also nehme ich an das das von Whiz-zarD richtig ist;

aber eine grundsätzlich unschöne sache ist das deine neue liste nicht aus deiner extra erstellten Klasse Person besteht sondern aus einem Array? (oder was new {String,String,String} auch immer erzeugt) ist. Du kannst also alle methoden etc die du für deine PersonenKlasse erstellst nicht darauf anwenden. Das heißt selbst wenn du new{Name,Vorname,GebDat,Wert} erstellen könntest wäre es oft geschickter trotzdem eine liste<Person> zu erstellen.

Es wird kein Array erzeugt, sondern ein anonymer Datentyp, der nur innerhalb der Methode verwendet werden kann.

Und ja, wenn in Person weitere Methoden sind (wovon ich aber abraten würde), könnte er sie nicht aufrufen, ohne erstmal aus dem anonymen Datentyp eine Person zu instanziieren.

[shao123]

Danke für eure Ideen. 

Das MoreLinq hatte ich auch schon im Netz entdeckt. Wir dürfen aber keine externen Libraries etc. verwenden, bzw. nur die, die durch eine bestimmte Abteilung freigegeben sind. Da wir eine Behörde sind, würde die Freigabe ewig dauern.

Ich hab eure Ansätze genommen, durchprobiert und habe mir dann eine eigene Lösung geschaffen. Ist zwar nicht die beste Lösung sicherlich. Hat mich aber zum Ziel geführt.

Hier mein Code: 

// Hier erst einmal ein Distinct auf Vorname, Nachname und Geburtsdatum 
var listDistinct = liste.Select(a => new { a.Vorname, a.Nachname, a.Geburtsdatum }).Distinct().ToList();
            var listCopy = new List<Person>(liste);
            var toRemove = new List<Person>();
            var tempResults = (dynamic)null;
//Hier wird meine Liste abgearbeitet um aus der ursprünglichen Liste den Abgleich zu machen, und ich erhalte die, die doppelt sind.
            for (int i = 0; i < liste.Count; i++)
            {
                 tempResults =  listDistinct.Find(item => item.Vorname.Equals(liste[i].Vorname) && item.Nachname.Equals(liste[i].Nachname) && item.Geburtsdatum.Equals(liste[i].Geburtsdatum));
                if(tempResults != null)
                {
                    listDistinct.Remove(tempResults);
                    liste.RemoveAt(i);
                    i--;
                   
                }
            }
// Final wird dann meine Liste mit den doppelten entfernt
            foreach(var item in liste)
            {
                listCopy.Remove(item);
            }

 

[arlegermi]

Ok, wenn du nichts externes nutzen darfst...

Von wievielen Elementen in der Liste reden wir hier und wie oft musst du die filtern?

Darf man dir Tipps zu deinem Code geben oder möchtest du im Augenblick nichts hören? Kann ich verstehen, wenn's dich jetzt nicht mehr juckt, aber man könnte da noch ein bißchen was dran drehen

[shao123]

In der Liste sind ca 20 - 50 Elemente.

Ja sehr gerne

Freue mich immer auf Verbesserungs,- und Optimierungsvorschläge

[Whiz-zarD]

vor 11 Minuten schrieb shao123:

Freue mich immer auf Verbesserungs,- und Optimierungsvorschläge

Wofür macht man denn ein Vorschlag, wenn du selbst einen Frickelcode zusammenschustert, den kein Schwein versteht?

Schreib doch ein Comparer, wie ich schon vorgeschlagen habe. Mehr brauchst du nicht und schon gar nicht tausend Listen, die dir den Speicher zumüllen.

Wenn man schon Kommentare schreiben muss, um den Code zu erklären, dann macht man in der Regel schon fast falsch.

Bearbeitet 13. Juni 2018 von Whiz-zarD

[shao123]

vor 10 Minuten schrieb Whiz-zarD:

Wofür macht man denn ein Vorschlag, wenn du selbst einen Frickelcode zusammenschustert, den kein Schwein versteht?

Schreib doch ein Comparer, wie ich schon vorgeschlagen habe. Mehr brauchst du nicht und schon gar nicht tausend Listen, die dir den Speicher zumüllen.

Wenn man schon Kommentare schreiben muss, um den Code zu erklären, dann macht man in der Regel schon fast falsch.

Sorry, kam mit dem Code nicht zurecht.

 

vor 10 Minuten schrieb Whiz-zarD:

Wenn man schon Kommentare schreiben muss, um den Code zu erklären, dann macht man in der Regel schon fast falsch.

Kommentare hab ich rein gemacht, damit Ihr versteht was ich gemacht habe. 

Mein alter Ausbilder hat immer gesagt, Code ohne Kommentare ist toter Code

[Whiz-zarD]

 

vor 54 Minuten schrieb shao123:

Mein alter Ausbilder hat immer gesagt, Code ohne Kommentare ist toter Code 

Sorry, aber dann hat er keine Ahnung.
Ein Code sollte im Idealfall wie ein Prosatext zu lesen sein. Wenn man dann anfängt Kommentare schreiben zu müssen, um den Code zu erklären, dann macht man was falsch. Zu mal, solche Kommentare irgendwann lügen, da der Code angepasst wird aber nicht der Kommentar.

Ein Kommentar sollte, wenn überhaupt, das "Wieso" erklären und nicht das "Was". Das "Was" erzählt schon der Code.

vor 54 Minuten schrieb shao123:

Sorry, kam mit dem Code nicht zurecht.

Und was ist an den Code nicht verständlich? Es ist eine simple Überprüfung, wann zwei Person-Objekte als gleich zu betrachten sind. Dafür wird ein Hashwert und eine Equals()-Methode benötigt. Der Hashwert ist im Grunde nichts weiter, als eine Zahl, die das Objekt repräsentiert. Es ist quasi eine Performanceoptimierung. Bei meinem Comparer gibt es aber noch einen Fehler. Die GetHashCode()-Methode berücksichtigt auch den Wert und daher funktioniert es nicht. Die XOR-Verknüpfung von Wert muss raus.

public class PersonComparer : IEqualityComparer<Person>
{
    public bool Equals(Person x, Person y)
    {
        if (x == null || y == null)
            return false;

         return x.Vorname == y.Vorname 
             && x.Nachname == y.Nachname 
             && x.Geburtsdatum == y.Geburtsdatum;
     }

    public int GetHashCode(Person obj)
    {
        return obj.Vorname.GetHashCode()
               ^ obj.Nachname.GetHashCode()
               ^ obj.Geburtsdatum.GetHashCode();
     }
}

Intern verwendet Distinct() eine Art HashSet, die aber speziell für LINQ entwickelt wurde. Distinct() vergleicht die Werte über einen Comparer. Wenn man keinen Comparer verwendet, wird intern ein DefaultComparer verwendet. Wie dieser genau arbeitet, weiß ich allerdings auch nicht.

Nicht desto Trotz, lässt sich nun ein eigenen Comparer implementieren. GetHashCode() ermittelt aus dem Objekt eine Zahl. Dazu gibt es etliche Artikel, wie die Methode zu implementieren ist. Die Empfehlung ist, von den jeweiligen Properties den Hashwert ermitteln (die Methode befindet sich am Objekt selber) und diese dann mit einem XOR zu verknüpfen. Damit bekommt man einen eindeutigen Hashwert. Das HashSet kann dann daraufhin sehr performant ermitteln, ob  beide Objekte gleich sind, da er er nur zwei Zahlen miteinander vergleichen muss. Zusätzlich gibt es noch die Equals()-Methode, die eine weitere Überprüfung darstellt, falls dann doch zwei Hashwerte gleich sind.

Du könntest auch einfach bei GetHashCode() eine 0 zurückgeben. Dann ist der Hashwert immer 0 und bei jedem Objektvergleich würde die Equals()-Methode ziehen. Diese Variante kann aber ggf. zu einer schlechten Performance führen.

public int GetHashCode(Person obj)
{
    return 0;
}

Dann macht der Code genau das, was du willst und du brauchst kein Boilercode, wo du zig Listen, mehre LINQ-Ausdrücke erstellst.

Bearbeitet 13. Juni 2018 von Whiz-zarD

[arlegermi]

Bei so wenig Elementen kann man auch ein wenig Performanz gegen Lesbarkeit tauschen. Man kann das ganze in einem Aufruf schreiben:

var distinct = liste.GroupBy(p => new { p.Vorname, p.Nachname, p.Geburtsdatum })
  		    .Select(g => g.First());

Du gruppierst erst nach den Attributen, die für dich die Eindeutigkeit ausmachen und nimmst dann immer das erste Element der Gruppe. (Eine Gruppe ist im Prinzip nichts anderes als ein Dictionary mit mehr als einem Wert pro Schlüssel.)

Was bei dir im Augenblick gar nicht berücksichtigt wird (ich weiß nicht, ob das für deinen Fall relevant ist), ist, welcher Personendatensatz genommen wird. Wenn du bspw. immer die Person mit dem höchsten Wert haben wolltest, könntest du das da oben leicht entsprechend anpassen:

var distinct = liste.GroupBy(p => new { p.Vorname, p.Nachname, p.Geburtsdatum })
                    .Select(g => g.OrderByDescending(p => p.Wert).First()).ToList();

Zu deinem Code:

Die Liste toRemove wird nirgends genutzt - weg damit.

Dein tmpResults musst du nicht mit dynamic deklarieren - du kannst einfach in der Schleife mit var arbeiten, wie du es an anderen Stellen auch schon machst.

Zudem finde ich deine Logik rund um das .Remove sehr anfällig für Fehler. Ich würde mich eigentlich immer davor hüten, den Schleifenzähler (i) innerhalb der Schleife zu manipulieren - das mag hier gut gehen, andere verwirrt's aber. Noch dazu manipulierst du die Liste, über die du gerade iterierst. Das geht mit einer for-Schleife zwar (mit einer foreach nicht!), macht die Sache aber auch nicht gerade übersichtlicher.

Im Allgemeinen würde ich etwas, wo ich mehrere Listen brauche, die aber irgendwie alles das gleiche enthalten und die ich dann irgendwie miteinander verschneiden muss, mindestens in eine eigene Methode auslagern, die ich dann vernünftig benenne (in deinem Fall sowas wie SelectDistinctPersons(IEnumerable<Person> persons)) viel eher aber noch versuchen, das ganze so zu programmieren, dass es völlig offensichtlich ist, was du da tust. Das kannst du auf verschiedenen Wegen erreichen - einen hat dir @Whiz-zarD gezeigt, ich finde meinen auch nicht verkehrt (den obersten mit dem DistinctBy).

[shao123]

Ich danke allen für Euren Support.

Morgen habe ich wieder mehr Zeit. Dann werde ich nochmals eure Codeschnipsel probieren.