Willkommen auf myCSharp.de! Anmelden | kostenlos registrieren
 | Suche | FAQ

Hauptmenü
myCSharp.de
» Startseite
» Forum
» Suche
» Regeln
» Wie poste ich richtig?

Mitglieder
» Liste / Suche
» Wer ist online?

Ressourcen
» FAQ
» Artikel
» C#-Snippets
» Jobbörse
» Microsoft Docs

Team
» Kontakt
» Cookies
» Spenden
» Datenschutz
» Impressum

Verschachtelten und redundanten Code vereinfachen (bezogen auf ein konkretes Beispiel)
321risiko
myCSharp.de - Member

Avatar #avatar-2832.png


Dabei seit:
Beiträge: 60
Herkunft: Bielefeld

Themenstarter:

Verschachtelten und redundanten Code vereinfachen (bezogen auf ein konkretes Beispiel)

beantworten | zitieren | melden

Wie kann ich den folgenden Code vereinfachen? Es geht mir hauptsächlich um den letzten Teil mit der Zuweisung an die Datasource-Eigenschaft.

using (FirebirdEmbedded fb = new FirebirdEmbedded())
{
   if (comboBox1.SelectedIndex == 0) {
      List<Category> g = fb.getCategories(true);

      // Es geht um dieses if
      if (g != null) {
         listBox1.DataSource = null;
         listBox1.Items.Clear();

         listBox1.DataSource = g;
      }
   }
   else if (comboBox1.SelectedIndex == 1) {
      List<Country> g = fb.getCountries(true);

      // Es geht um dieses if
      if (g != null) {
         listBox1.DataSource = null;
         listBox1.Items.Clear();

         listBox1.DataSource = g;
      }
   }
   else if (comboBox1.SelectedIndex == 2) {
      List<Language> g = fb.getLanguages(true);

      // Es geht um dieses if
      if (g != null) {
         listBox1.DataSource = null;
         listBox1.Items.Clear();

         listBox1.DataSource = g;
      }
   }
}
"Life is brutal and full of Zasadzkas"
private Nachricht | Beiträge des Benutzers
Wax
myCSharp.de - Member

Avatar #avatar-2276.jpg


Dabei seit:
Beiträge: 745
Herkunft: Dortmund

beantworten | zitieren | melden

Hi 321Risiko,

untersuch doch erstmal welche Zeilen Du mehrfach schreibst. Also welche immer gleich sind und diese würde ich schonmal aus den einzelnen if-Zweigen rausziehen und einmal ans Ende setzen.

MfG
wax
private Nachricht | Beiträge des Benutzers
Th69
myCSharp.de - Experte

Avatar #avatar-2578.jpg


Dabei seit:
Beiträge: 4196

beantworten | zitieren | melden

Hallo Bielefelder -)

Das "listbox.Items.Clear()" ist überflüssig, da die Zuweisung an die DataSource die Items selbständig neu setzt (und vorher löscht).

Und auch das Setzen der DataSource auf null ist nur notwendig, falls dasselbe Objekt nochmals zugewiesen wird (aber sich Änderungen an den einzelnen Listen-Einträgen ergeben haben). Wenn du jedoch immer wieder eine neue Liste erstellst (d.h. Zugriff auf die Firebird-Datenbank), dann reicht auch einfach


listBox.DataSource = fb.getCategories(true);
(und auch die Abfrage auf null sehe ich nicht als sinnvoll an, d.h. selbst dann sollte die Zuweisung erfolgen und die ListBox-Einträge gelöscht werden).

P.S. Außerdem könntest du noch den if-else-Block in eine switch-case-Anweisung ändern...
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Also ich würde hier mit Methoden arbeiten. Rein deswegen um die Lesbarkeit des Codes zu verbessern und dient auch der Wartbarkeit.

 
using (FirebirdEmbedded fb = new FirebirdEmbedded())
{
                if (comboBox1.SelectedIndex == 0)
                {
                    FillCategories(fb);
                }
                else if (comboBox1.SelectedIndex == 1)
                {
                    FillCountries(fb);
                }
                else if (comboBox1.SelectedIndex == 2)
                {
                    FillLanguages(fb);
                }
}

private static void FillLanguages(FirebirdEmbedded fb) {
            List<Language> g = fb.getLanguages(true);
            AddItemsToListBox(g);
          
}

private static void FillCountries(FirebirdEmbedded fb) {
            List<Country> g = fb.getCountries(true);
            AddItemsToListBox(g);
            
}

private static void FillCategories(FirebirdEmbedded fb) {
            List<Category> g = fb.getCategories(true);
            AddItemsToListBox(g);
          
}

private static void AddItemsToListBox(List<Object> g) {
           
           if(g!=null){
            listBox1.DataSource = null;
            listBox1.Items.Clear();

            listBox1.DataSource = g;
            }
}

Anstelle von List<Object> g (Parameter AddItemsToListBox) wäre hier ein BaseTyp für alle möglichen sicherlich besser.
Dieser Beitrag wurde 2 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
FZelle
myCSharp.de - Experte



Dabei seit:
Beiträge: 10084

beantworten | zitieren | melden

Alle List<T> sind auch IList, also ist es so deutlich kürzer:


using (FirebirdEmbedded fb = new FirebirdEmbedded())
{
	IList dataSource = null;
	switch(comboBox1.SelectedIndex)
	{
		case 0 : dataSource = fb.getCategories(true); break;
		case 1 : dataSource = fb.getCountries(true); break;
		case 2 : dataSource = fb.getLanguages(true); break;
	}
	listBox1.DataSource = dataSource;
}
Wobei es auch möglich wäre das get entweder in eine Factory auszulagern oder im DAL per parameter zu bekommen, dann wäre es:


using (FirebirdEmbedded fb = new FirebirdEmbedded())
{
	listBox1.DataSource = fb.GetList((ListTypeEnum)comboBox1.SelectedIndex);
}
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von FZelle am .
private Nachricht | Beiträge des Benutzers
herbivore
myCSharp.de - Experte

Avatar #avatar-2627.gif


Dabei seit:
Beiträge: 52329
Herkunft: Berlin

beantworten | zitieren | melden

Hallo dani_m,

das sieht mir wie ein Eigentor aus. :-) Zwar ist dein Ansatz, um die Redundanzen zu vermeiden, schon richtig, aber am Ende hast du genauso viel Code oder sogar etwas mehr Code als am Anfang. Das kann ja irgendwie nicht das Ziel sein. :-) Dabei geht es sehr viel kürzer und übersichtlicher. Der Code schnurrt zusammen zu:

using (FirebirdEmbedded fb = new FirebirdEmbedded()) {
   if (comboBox1.SelectedIndex == 0) {
      listBox1.DataSource = fb.getCategories(true);
   } else if (comboBox1.SelectedIndex == 1) {
      listBox1.DataSource = fb.getCountries(true);
   } else if (comboBox1.SelectedIndex == 2) {
      listBox1.DataSource = fb.getLanguages(true);
   }
}

Oder eben mit einem switch, wie FZelle es gezeigt hat.

herbivore
private Nachricht | Beiträge des Benutzers
321risiko
myCSharp.de - Member

Avatar #avatar-2832.png


Dabei seit:
Beiträge: 60
Herkunft: Bielefeld

Themenstarter:

beantworten | zitieren | melden

Vielen Dank für eure Antworten. Stand da ein bißchen auf dem Schlauch... Hab das jetzt so vereinfacht:


using (FirebirdEmbedded fb = new FirebirdEmbedded()) {
  switch (comboBox1.SelectedIndex) {
    case 0: listBox1.DataSource = fb.getCategories(true);
      break;
    case 1: listBox1.DataSource = fb.getCountries(true);
      break; 
    case 2: listBox1.DataSource = fb.getLanguages(true);
      break;
  }
}
"Life is brutal and full of Zasadzkas"
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Hallo Herbivore,

Klar ist das Möglich aber ich würde hier definitiv mit ausgelagerten Methoden arbeiten. Erstens sind die Methoden Testbar, was in deiner Version nicht zu machen ist, zweitens der Compiler arbeitet später mit Inline Methoden somit spar ich mir unnötigen Code zur Laufzeit.
Drittens was ist wenn du später etwas nicht anzeigen willst was mir z.B. von fb.getCountries zurückkommt?

Viertens würd ich definitiv nicht über eine DataSource arbeiten. Wer sagt mir den das ich immer alle Felder anzeige die von der DB zurückkommen. Nichts ist so sicher wie die Veränderung in der Softwareentwicklung.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
herbivore
myCSharp.de - Experte

Avatar #avatar-2627.gif


Dabei seit:
Beiträge: 52329
Herkunft: Berlin

beantworten | zitieren | melden

Hallo dani_m,

zu 1. Die Testbarkeit ist auch bei mir gegeben, da die wichtigen Methoden wie getCategories, getCountries ja weiterhin eigenständige Methoden sind und damit eigenständig testbar.

zu 2. Laufzeit-Unterschiede spielen hier keine Rolle und sind deshalb kein Entscheidungskriterium, weder in die eine noch in die andere Richtung.

zu 3. Wenn man etwas von getCountries nicht anzeigen will, sind bei dir und bei mir genauso viele oder wenige Code-Änderungen erforderlich. Das nimmt sich also nichts.

zu 4. Du arbeitest ja definitiv mit einer DataSource. Daher ist der Einwand etwas abwegig. Was passiert, wenn du nicht alle Felder anzeigen willst, haben wir schon bei Punkt 3 besprochen.

Sicher ist nichts ist so sicher wie die Veränderung in der Softwareentwicklung. Deshalb ist es auch gerade so wichtig, die Codemenge die unnötig zu erhöhen. Ich halte meine 10 Zeilen für wesentlich wartbarer als deine 42.

herbivore
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Hallo Herbivore,

zu 1. Die Testbarkeit ist hier zwar gegeben solang sich nix ändert ob ich alles anzeigen will oder nicht.

zu 2. Laufzeit-Unterschiede spielen natürlich eine Rolle. Bei der Entwicklung muss man schon aufpassen was guter Code und was schlechter Code ist.
Switch-Anweisungen werden beispielsweise niemals Inline vom Compiler gemacht. If-Else schon solange die Methode nicht größer als 32 bytes beträgt.

zu 3. Klar aber bei mir ist die Änderung nur in der Methode

private static void FillCountries(FirebirdEmbedded fb) {
            List<Country> g = fb.getCountries(true);
            
            SelectOnlyCountriesWithStartName("A",g);
            
            AddItemsToListBox(g);


}

zu 4. Ich habe hier nur die DataSource verwendet da Sie vom Ersteller vorgegeben war. Ich würds niemals verwenden weil ich weiß wie oft sich ein Kunde umentscheidet das er das oder das jetzt angezeigt haben will.

Deine 10 Zeilen sind hier solang wartbarer solang sich nix ändert aber wehe der Kunde will was anderes dann musst unweigerlich in Methoden aufbrechen.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
winSharp93
myCSharp.de - Experte

Avatar #avatar-2918.png


Dabei seit:
Beiträge: 6155
Herkunft: Stuttgart

beantworten | zitieren | melden

Hallo zusammen,

Ziel wäre es ja eigentlich, sämtliche ifs und switches zu eliminieren.

Leider habe ich gerade keine Möglichkeit, das zu testen, aber folgender, schemenhafter Code sollte funktionieren:


public sealed class CustomComboboxItem
{
   public string Name { get; private set;} //Initialisierung im Konstruktor
   public Func<FirebirdEmbedded, object /*?*/> LoadDataCallback { get; private set; } //Initialisierung im Konstruktor

   public object LoadData(FirebirdEmbedded db)
   {
       return this.LoadDataCallback(db);
   }
}

//...
this.Combobox.DisplayMember = "Name";
this.Combobox.Items.Add(new CustomComboboxItem("Categories", db => db.getCategories(true));
this.Combobox.Items.Add(new CustomComboboxItem("Languages", db => db.getLanguages(true));
//...

using (FirebirdEmbedded db= new FirebirdEmbedded()) 
   listBox1.DataSource = this.Combobox.SelectedItem.LoadData(db);
private Nachricht | Beiträge des Benutzers
FZelle
myCSharp.de - Experte



Dabei seit:
Beiträge: 10084

beantworten | zitieren | melden

@dani_m:
Zitat
Deine 10 Zeilen sind hier solang wartbarer solang sich nix ändert aber wehe der Kunde will was anderes dann musst unweigerlich in Methoden aufbrechen.

Man sollte sein aktuelles Design niemals auf dem basieren was evtl mal alles kommen könnte, sondern nur auf dass was kommen wird, und dabei immer YAGNI und KISS bedenken.

Mit den heutigen Refactoring Möglichkeiten und guter Testabdeckung spart es dir heute Zeit und Komplexität.
Wenn der Kunde es irgendwann anders haben möchte, muss er ja sowieso den Aufwand bezahlen.
Ihn aber von Anfang an unnötige Komplexität bezahlen lassen ist nicht gut.
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

@FZelle

Man sollte sein aktuelles Design niemals auf dem basieren was evtl mal alles kommen könnte, sondern nur auf dass was kommen wird --> Erweiterbarkeit??? Ist die Erweiterbarkeit nicht der Grundsatz der Objektorientierten Programmierung? In alten Prozedualen Sprachen hätte ich mir das erwartet aber nicht in einer Objektorientierten.

Was ist daran bitte nicht Simple? Ich brechs lediglich in Methoden auf. Somit bin ich erweiterbar was in den anderen Version nicht möglich ist.

In software design, change is constant...
private Nachricht | Beiträge des Benutzers
herbivore
myCSharp.de - Experte

Avatar #avatar-2627.gif


Dabei seit:
Beiträge: 52329
Herkunft: Berlin

beantworten | zitieren | melden

Hallo dani_m,

zu 1. Das ist eine reine Behauptung.

zu 2. Also, sorry, bei Code, hinter dem sich vermutlich Datenbankzugriffe verbergen und in dem die Methodenaufrufe nur *einmal* pro Benutzeraktion erfolgen, müssen wir wirklich nicht über Inlining reden. Da zäumst du das Pferd wirklich von der falschen Seite auf. Wenn du dir in solchen Fällen Gedanken über Inlining machst, bist du ein "premature optimization is the root of all evil"-Kandidat.

zu 3. Bei mir ist es auch nur eine Änderung in einer Methode.

zu 4. Ob man DataSource verwendet oder nicht, hat doch nichts damit zu tun, ob und wie die Daten vorher selektiert wurden.

Eigentlich möchte ich gar nicht weiter diskutieren, weil das einfach so extrem abwegig und unsinnig ist, was du schreibst. Mal abgesehen von dem Verstoß gegen YAGNI und KISS.

herbivore
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Und gegen was verstößt dein Ansatz?

Single Responsibility Principle
Open/Closed Principle

Klar hast du nur eine Änderung an der Methode weil du nur eine Methode hast.
Diskutieren ist ja wohl erlaubt. Ich lass mich auch eines besseren beleheren.

Aber Keep it Simple bedeutet nicht das ich nur eine Methode habe in der ich mehrere Verantwortlichkeiten habe.
private Nachricht | Beiträge des Benutzers
herbivore
myCSharp.de - Experte

Avatar #avatar-2627.gif


Dabei seit:
Beiträge: 52329
Herkunft: Berlin

beantworten | zitieren | melden

Hallo dani_m,

das Single Responsibility Principle bezieht sich zumindest im engeren Sinne auf Klassen und nicht auf Methoden. Willst du jetzt auch noch anfangen, die winzigen Methoden auf unterschiedliche Klassen aufteilen, damit das SRP für alle beteiligten Klassen eingehalten ist? Damit es noch unübersichtlicher wird? Ich finds, wie gesagt abwegig.

Davon abgesehen sehe ich das SRP auch bei mir nicht verletzt. Zumindest wenn wir mal davon ausgehen, dass es Aufgabe des GUIs ist zu bestimmen, was angezeigt werden soll.

Das Open/Closed Principle bezieht sich auf Vererbung. Die ist doch hier gar nicht im Spiel.

herbivore
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden

Ich sehe das ähnlich wie FZelle. Eine Factory wäre hier durchaus sinnvoll, da man so den unschönen Seiteneffekt des Befüllens der ComboBox aus der Methode zum Auswählen des korrekten Datensatzes entfernen könnte. Wenn wir hier schon über KISS und YAGNI reden, dann würde ich hier auch gerne noch "side effects considered harmful" einbringen. Ich weiss, dass in der OOP selten darauf geachtet wird, wo man andere Objekte verändert, dennoch sollte man sinnvoll trennen. Seiteneffekte die sich auf die GUI beziehen haben nichts in der Logik verloren und vice versa.
private Nachricht | Beiträge des Benutzers
FZelle
myCSharp.de - Experte



Dabei seit:
Beiträge: 10084

beantworten | zitieren | melden

@dani_m:
Nur weil Du die CNC Maschine verstanden hast, muss sie nicht immer benutzt werden,manchmal reicht auch ein Schraubendreher.
Zitat
In software design, change is constant...
Du hast mein Posting also nicht gelesen !!
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Wie würdest du denn dieses Entwickeln? Test First? Somit müsstest du kleine Methoden schreiben. Testbare kleine Methoden. Wenn du schon mit KISS und YAGNI arbeitetst dann ist das sicherlich XP. XP setzt Tests vorraus. Kleine prägnante Methoden...

Single Responsibility bezieht sich in der Definition auf Klassen ja aber trotzdem sollte ich nicht in einer Methode zwei Grundlegende Verantwortlichkeiten mischen.

Open/Closed Principle besagt nur das ein System geschlossen vor Veränderungen sein muss und offen für Erweiterungen. Des heißt nicht das des nur mit Ableitung geht --> Favour Composition over Inheritance.
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden

Zitat von dani_m
Wenn du schon mit KISS und YAGNI arbeitetst dann ist das sicherlich XP.

Was haben denn KISS und YAGNI mit XP zu tun? Das sind grundlegende Prinzipien. XP ist eine Vorgehensweise, in der auch auf so etwas geachtet wird, aber nur weil jemand KISS und YAGNI beherzigt heisst es noch lange nicht, dass er/sie XP betreibt.
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Wenn ihr nur die zwei Prinzipien befolgen wollt na gut. Ich halt mich an ein bisschen mehr und ja mein code ist länger aber sagen nicht auch die Jungs von GoF das der Code durch Design Patterns länger wird dafür umso besser, lesbar, wartbar, erweiterbar...

Es gibt leider mehr als nur 2 Prinzipien die man beachten sollte auch wenn man nicht XP macht. Wenn einer hier aber schon auf Prinzipien herumhacken will na gut dann mach ich das auch. Test First Ansatz und da fängt man unweigerlich mit kleinen prägnanten Methoden an. So und was ist daran bitte nicht Simple? Methoden die mehr als 5 Zeilen Code haben ist schon zuviel. Das kann keiner mehr auf einen Blick erfassen. Oder schreibt ihr Methoden die Länger sind? Dann brauch ich gar ned mit Prinzipien zu diskutieren anfangen.

Uncle Bob (ObjectMentor) sorry ich hab geglaubt deine Prinzipien sind wichtig und sollten beachtet werden. Hier habe ich erfahren es gibt nur 2 Prinzipien die ich befolgen soll...
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
herbivore
myCSharp.de - Experte

Avatar #avatar-2627.gif


Dabei seit:
Beiträge: 52329
Herkunft: Berlin

beantworten | zitieren | melden

Hallo dani_m,

alle Prinzipien können letztlich nur eine Orientierung sein. Das sieht man ja auch daran, dass es Situationen gibt, in denen verschiedene Prinzipien miteinander konkurrieren. Um das eine Prinzip einzuhalten muss man ggf. gegen ein anders verstoßen. Letztlich ist es immer eine Frage der Abwägung. Die fällt hier m.E. ziemlich klar aus. In welche Richtung habe ich ja schon gesagt und begründet.

Ich finde es schon eine Leistung, den extrem redundanten Code von 321risiko so umzuschreiben, dass zwar die Redundanzen raus sind, der Code aber trotzdem länger ist als vorher. Meines Erachtens kosten die zusätzlichen Abstraktionsebenen mehr als die nutzen. Abstraktion hat unbestreitbare Vorteile, aber sie bringt eben fast immer auch zusätzliche Indirektion. Indirektion kostet uns Menschen leider zusätzlichen Aufwand, wenn man verstehen will, wie die Abläufe sind. Bei der Wartung muss man die Abläufe oft verstehen, um die beste Stelle für eine bestimmte Änderung zu finden. Im Ergebnis bringt Abstraktion deshalb leider nicht immer ein Profit, weil sie eben auch Kosten verursacht. Leider eben nicht nur bei der erstmaligen Erstellung, sondern auch bei der Wartung.

Mag sein, dass sich das nicht überzeugt. Mich überzeugen deine Argumente auch nicht. :-) Ich habe eben gemerkt, dass mir schon ein einziger Blick auf deinen und meinen Code reicht, um zu sehen, was ich wartbarer empfinde. Insofern können wir uns die Diskussion um Prinzipien eigentlich auch sparen. Denn die Prinzipien führen - da man sie immer auslegen und gegeneinander abwägen muss - zumindest in diesem Fall - am Ende sowieso zu dem, was einem von Anfang an besser gefallen hat.

Oder anders gesagt, wir haben hier die ganze Zeit versucht, nachträglich Argumente zu suchen, die genau zu dem Ergebnis führen, das man von Anfang das gewünschte ist. Das mag wenig professionell erscheinen, ist aber wohl nur ein Ausdruck der akkumulierten professionellen Erfahrungen. An manchen Stellen, weiß man eben auf Anhieb, was besser ist, selbst wenn es sowohl Argumente dafür wie dagegen gibt. Deshalb fechten einen die Argumente, die dagegen sprechen, auch nicht an. Das bedeutet aber auch, dass wir uns eine weitere Diskussion sparen können.

herbivore
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Sind das jetzt deine Argumente? Mein Code beseitigt die Redundanz wie du selbst schon sagtest. Er ist Simple. Jeder Entickler kann ihn lesen. Da es kleine Methoden sind mit prägnanten Namen die jeder auf Anhieb lesen kann. Ich weiß echt nicht was daran bitte schlecht ist. Performance ist auch besser, da der Compiler hier inline Methoden draus macht wie ich weiter oben schon erklärt habe. Ich kann an einer Stelle z.B. ändern das ich die Daten jetzt in einer DataGridView anzeigen will oder in einem TreeView.
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden

Zitat von dani_m
Wenn ihr nur die zwei Prinzipien befolgen wollt na gut. [...] Uncle Bob (ObjectMentor) sorry ich hab geglaubt deine Prinzipien sind wichtig und sollten beachtet werden. Hier habe ich erfahren es gibt nur 2 Prinzipien die ich befolgen soll...


Also erstmal kannst du dir deinen Sarkasmus sparen. Zweitens habe ich mit keinem Wort gesagt, dass es nicht sinnvoll ist XP zu betreiben. Ganz im Gegenteil, ich bin ein absoluter Befürworter von TDD, BDD und Pair Programming. Allerdings halte ich es für ein wenig blauäugig zu sagen, dass man es IMMER benutzen muss und egal was man tut auch ja auf Pattern achten sollte. Außerdem wie ich schon sagte ist es einfach nicht richtig aufgrund 2 Prinzipien auf eine gesamte Vorgehensweise einer Person zu schließen.

Wie du schon sagtest geht es im TDD bzw. BDD auch darum möglichst kurze Methoden zu schreiben. Das stimmt, allerdings gilt es auch herbei darauf zu achten nicht mehr Code zu produzieren, als unbedingt nötig. Siehe Red-Green-Refactor Cycle. Du schreibst einen Test, lässt ihn fehlschlagen, schreibst genau so viel Code, dass der Test erfolgreich durchläuft und refaktorisierst dann unnötigen Code raus. In deinem Fall hast du aber unnötig viel Code hergezaubert. Das widerspricht im eigentlichen Sinne dem Red-Green-Refactor Cycle.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von Corpsegrinder am .
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Genau das macht man aber bei Pair Programming. Man achtet gegenseitig auf guten qualitativ hochwertigen code. Das die Regeln eingehalten werden usw. das ist der Sinn von Pair Programming. Hier würde ich dir mal das Buch von Uncle Bob (Robert C. Martin) empfehlen (Agile Principles, Patterns and Practices in C#). Hier wird eine Beispielaufgabe mitttels Pair Programming gelöst und da stellen sich die Entwickler gegenseitig immer die Frage ist das Sinnvoll was du da implemtierst. Denk an die Verantwortlichkeiten usw...

Qualitativ hochwertiger Code wird nicht mehr mittels Lines of Code gemessen sondern mittels Codemetriken, Abhängigkeiten usw. Wie würdest du zu den anderen Versionen einen Test schreiben? Das würde mich brennend interessieren.

Ich habe in meiner Version die Redundanzen beseitigt. Das war die Aufgabe und die hab ich gelöst.

Ich weiß wie Test First funktioniert. Ich arbeite mit diesem Ansatz. Kannst du das Verhalten einer Methode da Testen mittels Mocken also ich find in den anderen Versionen keinen Ansatz das zu testen.

Wenn du mir eine brauchbare TestMethode zeigst für die anderen Versionen dann bin ich ruhig.

@herbivore zu deinem Statement
Zitat
Abstraktion hat unbestreitbare Vorteile, aber sie bringt eben fast immer auch zusätzliche Indirektion. Indirektion kostet uns Menschen leider zusätzlichen Aufwand, wenn man verstehen will, wie die Abläufe sind

Ja es hat Vorteile und die hab ich hier aufgezeigt. Die Abläufe lassen sich durch aussagekräftige Namen der Methoden leichter verstehen. Die Methoden sind bzgl Verantwortlichkeiten getrennt. Also alles in allem leichter Verständlich. Diesen Code kann jeder Entwickler lesen. Er weiß sofort was passiert hier, durch aussagekräftige Namen. Wie du auch sagtest hab ich in der Version die Redundanzen beseitigt. Das war auch die Aufgabe. Wie ich oben schon sagte wird die Qualität von Code nicht mittels Lines of Code gemessen.
Dieser Beitrag wurde 2 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden


Ganz einfach... wenn ich die von FZelle und mir vorgeschlagene Variante mit der Factory nehme schreibe ich mir einfach Test-Cases für die 3 verschiedenen Auswahlmöglichkeiten. Was ist daran nicht testbar?


// Angenommen ich habe in der Datenbank die Länder
// Deutschland, England und Spanien gespeichert,
// so erwarte ich, dass ich eine Liste dieser Länder
// erhalte, wenn ich DataFactory.getData(Categories.Country)
// aurufe
def testGetCountries {
  var countries = List("Deutschland", "England", "Spanien")
  var dbCountries = DataFactory.getData(Categories.Country)
  assertEqual(countries.size, dbCountries.size)
  assert(dbCountries.containsAll(countries))
}

Das ist nur schnell hin gehackter Pseudocode, aber sollte zeigen, dass es sich durchaus testen lässt.
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von Corpsegrinder am .
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

So und wie testest du ohne Datenbank???

Zwei Asserts in einer TestMethode ist nicht gut. Wenn herbivore schon meckert wegen inderektion warum nehmt ihr dann eine Factory her. Statische Factory - lieber herr gesangsverein da schreib mal deinen Test dazu.

Kommt jetzt der Ansatz das man statische Methoden testen kann?

Außerdem ist das nicht Testgetrieben entwickelt.

Das was du da als einen Testfall beschreibst sind mehere Testfälle.

List beinhaltet mehrere Länder -> 1. Testfall
List beinhaltet nur ein Land -> 2. Testfall
List beinhaltet eine leere Liste -> 3. Testfall

Außerdem zwei listen kannst du mittels SequenceEqual vergleichen.

Außerdem einfach hin gehackt wenn ich sowas schon höre...
Dieser Beitrag wurde 1 mal editiert, zum letzten Mal von dani_m am .
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden


Klar ist das Test-Driven. Ich habe keine Zeile der Implementierung geschrieben, bevor der Test fertig war ;-). Außerdem ist das ein Beispiel und stellt keinerlei Anspruch auf Vollständigkeit. Und ich habe geschrieben, dass es Pseudocode ist. Welche Methoden man im .Net Framework zur Verfügung hat ist hierbei vollkommen irrelevant. Aber ich merke schon, dass man mit dir nicht sachlich diskutieren kann.
private Nachricht | Beiträge des Benutzers
dani_m
myCSharp.de - Member



Dabei seit:
Beiträge: 13

beantworten | zitieren | melden

Das ist nicht Test-Driven. Wenn es TestDriven wäre würdest du wissen das ne statische Factory nicht Testbar ist. Du kannst ne statische Methode nur Testen was geht rein und was kommt raus. Das Verhalten was du mittels Mock machen würdest kannst du bei einer statischen Methode nicht testen.

Mit mir kann man sachlich diskutieren. Nur wenn ich schon höre ich hab das schnell hingehackt ist das nicht sachlich.

Was ist wenn einer deiner asserts fehlschlägt (wenn du jemand bist der in einer TestMethode mehrere Asserts schreibt)? Wie findest du raus welcher es ist?
private Nachricht | Beiträge des Benutzers
Corpsegrinder
myCSharp.de - Member



Dabei seit:
Beiträge: 416

beantworten | zitieren | melden


Der Sinn dieser Factory Methode ist es einen Parameter "Kategorie" anzunehmen und eine Liste mit Daten zurückzuliefern. Keine Seiteneffekte. Das ist absolut testbar. Außerdem denkst du viel zu sehr in C#... Ich benutze z.Zt. so gut wie nie C#, sondern andere Sprachen, die das Konzept der Objektorientierung strikter umsetzen, wie z.B. Ruby oder Scala. Da gibt es keine statischen Klassen, von Daher kann man da auch wunderbar Mocks benutzen. Unterstelle anderen nicht irgendetwas aufgrund von Indizien die du dafür zu sehen glaubst.
private Nachricht | Beiträge des Benutzers