[C#-Spiel] Programmierherrausforderung

[Blackhawk50000]

Hallo freunde. wenn ihr lust habt, habe ich hier eine kleine Aufgabe für euch...

Ich habe eine sehr gut funktionierende Methode zum Filtern von einem List<Object> object geschrieben.

Ein kollege sagte zu mir, es sei wirklich schlecht zu warten und könnte fast garnicht funktonieren....

Wenn ihr wollt, schreibt den Code doch mal um =)

Fragen bezüglich weiteren informationen die vielleicht fehlen, beantworte ich gerne...

/// <summary>
        /// Mit der Schnellsuche auf Du und Du
        /// Ein Bestseller von mir.
        /// 
        /// In dier Methode wird ein Lambda ausdruck zusammengebaut der alle Kombinationen von 5 Checkboxen berücksichtigt.
        /// Diese 5 Checkboxen teilen sich in 2 Gruppe auf.
        /// 
        /// Gruppe 1: Zahlungsart
        /// Gruppe 2: Offener Posten
        /// 
        /// Die Checkboxen für die schnellsuche und die Dazugehörigen eigenschaftswerte sind:
        /// 
        ///     Lastschriftzahler       ----> ZahlungsArt = 1
        ///     Rechnungszahler         ----> ZahlungsArt = 2
        /// 
        ///     Guthaben                ----> Offener Posten kleiner 0
        ///     Keine offenen Posten    ----> Offener Posten = 0
        ///     Forderung               ----> Offener Posten > 0
        ///     
        /// Jede der Checkboxen hat eine eigene Abfrage, die einen Teil des Labdaausdruckes bildet.
        /// Diese Abfragen sind mit der Kurzen IF schreibweise geschrieben um auf den Checked-Status der Checkbox entsprechend zu reagieren.
        /// 
        /// Beispiel normale if-abfrage
        /// 
        ///     if(variable == true)
        ///     {
        ///         string = 1;
        ///     }
        ///     else
        ///     {
        ///         string = 2;
        ///     }
        /// 
        /// Die gleiche Abfrage in Kurzer schreibweise:
        /// 
        ///     variable ? string = 1 : string = 2;
        ///     
        /// Diese Abfragen können beliebig verschachtelt werden. Im Fall der Schnellsuche, wird Jedes Teil der Gruppe mit jedem Teil der Gruppe verschachtelt.
        /// 
        /// 
        /// </summary>
        private void SetQuickSearch()
        {
            if (_zahler != null && _zahler.Count > 0)
            {

                _zahlerSuchErgebnis = _zahler.Where
                    (
                        x =>
                        (
                            (_rechnungsZahler ? x.ZahlungsArt == 1 : (_lastschriftZahler ? x.ZahlungsArt == 2 : false))
                        ||
                            (_lastschriftZahler ? x.ZahlungsArt == 2 : (_rechnungsZahler ? x.ZahlungsArt == 1 : false))
                        )
                        &&
                        (
                            (_guthaben ? x.OffenerPosten < 0 : (_keineOffenenPosten ? x.OffenerPosten == 0 : (_forderung ? x.OffenerPosten > 0 : false)))
                        ||
                            (_keineOffenenPosten ? x.OffenerPosten == 0 : (_guthaben ? x.OffenerPosten < 0 : (_forderung ? x.OffenerPosten > 0 : false)))
                        ||
                            (_forderung ? x.OffenerPosten > 0 : (_guthaben ? x.OffenerPosten < 0 : (_keineOffenenPosten ? x.OffenerPosten == 0 : false)))
                        )
                        &&
                        (
                            (_mahnstatusGleichNull ? x.Mahnstatus == 0 : x.Mahnstatus == 1)
                        )
                    ).ToList();

                ABR_LadeZahlerCompleteEvent(_zahlerSuchErgebnis, "");
            }
        }

 

[Nico Graichen]

Hi

Also die Arbeit wird dir keiner abnehmen!
Aber:
3 Punkte im Code schrenken die Wartbarkeit ein:
1. die Lambda-Expression, noch dazu wenn sie so riesig und komplex ist
2. die Nutzung des ?:-Operators, den du hier exzesiv nutzt
3. die Kombination beider

Kurz gesagt. Sollte in der Bedingung ein Fehler sein, ist es fast unmöglich diesen zu finden, da es keine Möglichkeit gibt, die einzelnen Bestandteile der Bedingung zu analysieren (per Debugger)

 

[Spyke]

Soweit ich seh geht es wohl um Mahnungen der Debitoren.
Aber ansonsten empfinde ich es auch eher als schwer zu lesen und wie schon gesagt schwer zu kontrollieren.

 

[Nico Graichen]

Die Nameskonvention ist übrigens auch weit weg von allen Geflogenheiten

 

[Blackhawk50000]

Hm, also die namensgebung ist leider nicht auf meinem mist gewachsen. Die ist in der Firma vorgegeben und ich muss mich dran halten. =)

zweitens, Mahnungen der Debitoren**** Nicht ganz. es geht einfach nur um eine schnellsucher von personen nach kriterien.

Um Debitoren kümmere ich mich in einem Jahr =)

an Nico: Die Nutzung des ? Operators ist in diesem fall leider unumgänglich da eine lange if-schreibweise nicht innerhalb eines lambda ausdruckes unterstüzt wird.

die lambda-expression ist wirklich lang da gibt es nichts zu bestreiten, allerding geht es hier auf um die wahllose kombination von 5 schnellsuchkriterien.

und keine angst, hier geht es nicht darum, dass mir jemand arbeit abnimmt. ich wolltedas einfach nur mal just 4 fun rein schreiben.

 

[VScan]

Hey,

also meiner Meinung nach (die vielleicht nicht sehr wichtig ist ^^) ist der Kode sehr sauber.

Wie würde denn sowas sonst aussehen, wenn ich 5 Kriterien habe, die zu prüfen sind?

Klar kann man sowas leserlicher schreiben, aber ich denke, dass Dein Chef erstma' eine Suchmaschine benutzen musste, um den Kode zu verstehen, was Ihm wahrscheinlich nicht gepasst hat. ^^

Insgesamt würde der Kode viel mehr Zeilen haben, wenn man es "leserlicher" schreiben würde, ausserdem, wer einen Bildschrim hat, der breit genug ist, kann es auch nebeneinander schreiben, was ich persönlich bevorzuge, naja, solange es nicht über den Rand reicht. ;-)


Viele Grüße

VScan


Edit:
Verstehe auch nicht, was es an der Namensgebung zu meckern gibt, ich hab schon die ungewöhnlichsten Sachen gesehen, da sieht das doch ganz ok aus.
Ausserdem fehlt nicht viel zur MS-Namenskonvention, die einfach mit "m_" anfängt für "MemberAttributes", also ist das eigentlich ganz ok.

 

[Spyke]

Wie würde denn sowas sonst aussehen, wenn ich 5 Kriterien habe, die zu prüfen sind?

Dafür müsste man das Objektmodell genauer kennen, es sieht halt erstmal komplizierter aus als es sein müsste.

 

[Nico Graichen]

an Nico: Die Nutzung des ? Operators ist in diesem fall leider unumgänglich da eine lange if-schreibweise nicht innerhalb eines lambda ausdruckes unterstüzt wird.

Das If-Else innerhalb von Lambda nicht funktioniert weiß ich. Das legitimiert aber noch nicht die mangelnde Wartbarkeit des Codes!

die lambda-expression ist wirklich lang da gibt es nichts zu bestreiten, allerding geht es hier auf um die wahllose kombination von 5 schnellsuchkriterien.

Dafür ist LINQ aber nicht gemacht. Ich glaub fast, dass du mit einer Schleife und If-Else schneller zur Runtime bist! Falls du hier auf eine DB zugreifst wäre ein optimierter Query hier auch besser. LINQ ist nicht immer das beste Mittel um ein Ziel zu erreichen! Aber einer gewissen Komplexität wird es langsam.

und keine angst, hier geht es nicht darum, dass mir jemand arbeit abnimmt. ich wolltedas einfach nur mal just 4 fun rein schreiben.

Der erste Post lies halt auch eine andere Interpretation zu

Trotzdem: Die Netiquette gilt weiterhin! Also auch mal die Shift-Taste nutzen!