Ich bin zur Zeit, unter anderem damit beauftragt ein Projekt für einen Kunden zu überarbeiten und zu optimieren.
In diesem Zusammenhang bin ich Heute Morgen auf ein Stück Quellcode gestoßen der mich sofort an die früher (so vor 20 Jahren) sehr üblichen Spaghetti Code Programme erinnert hat.
Ich zeige hier einfach mal den Code dieser Methode wie ich sie vorgefunden habe und anschließend die von mir leicht überarbeitete Methode.
Die Methode wird übrigens dazu verwendet verschiedene Button ein und auszublenden, sowie in Abhängigkeit von Datenfeldern einer Datenbank den Button mit einem Image zu verschönern und entsprechende Datums und Uhrzeiten anzuzeigen oder auszublenden.
Ich denke anhand der nachfolgenden Abbildungen kann man sich ungefähr vorstellen was die Methode macht.
Hier der doch etwas unübersichtliche Code
#region Ankunft if (entladung.AnkunftLKW.HasValue) { dtpAnkunftDate.Visible = true; dtpAnkunftTime.Visible = true; } else { dtpAnkunftDate.Visible = false; dtpAnkunftTime.Visible = false; } #endregion #region Einfahrt if (entladung.EinfahrtLKW.HasValue) { dtpEinfahrtDate.Visible = true; dtpEinfahrtTime.Visible = true; } else { dtpEinfahrtDate.Visible = false; dtpEinfahrtTime.Visible = false; } #endregion #region Entladung Start if (entladung.EntladeStart.HasValue) { dtpEntladungStartDate.Visible = true; dtpEntladungStartTime.Visible = true; btnEntladungStart.ImageIndex = 0; if (entladung.EntladeEnde.HasValue) { btnEntladungStart.Enabled = false; } else { btnEntladungStart.Enabled = true; } } else { dtpEntladungStartDate.Visible = false; dtpEntladungStartTime.Visible = false; } #endregion #region Entlade Ende if (entladung.EntladeEnde.HasValue) { dtpEntladungEndeDate.Visible = true; dtpEntladungEndeTime.Visible = true; btnEntladungEnde.ImageIndex = 0; btnEntladungEnde.Enabled = !entladung.AusfahrtLKW.HasValue; } else { if (entladung.EntladeStart.HasValue) { btnEntladungEnde.Enabled = true; } else { btnEntladungEnde.Enabled = false; btnEntladungEnde.ImageIndex = -1; } dtpEntladungEndeDate.Visible = false; dtpEntladungEndeTime.Visible = false; } #endregion #region Ausfahrt if (entladung.AusfahrtLKW.HasValue) { dtpAusfahrtDate.Visible = true; dtpAusfahrtTime.Visible = true; } else { dtpAusfahrtDate.Visible = false; dtpAusfahrtTime.Visible = false; } #endregion
Hier der Optimierte Code
dtpAnkunftDate.Visible = entladung.AnkunftLKW.HasValue; dtpAnkunftTime.Visible = entladung.AnkunftLKW.HasValue; dtpEinfahrtDate.Visible = entladung.EinfahrtLKW.HasValue; dtpEinfahrtTime.Visible = entladung.EinfahrtLKW.HasValue; dtpEntladungStartDate.Visible = entladung.EntladeStart.HasValue; dtpEntladungStartTime.Visible = entladung.EntladeStart.HasValue; dtpEntladungEndeDate.Visible = entladung.EntladeEnde.HasValue; dtpEntladungEndeTime.Visible = entladung.EntladeEnde.HasValue; dtpAusfahrtDate.Visible = entladung.AusfahrtLKW.HasValue; dtpAusfahrtTime.Visible = entladung.AusfahrtLKW.HasValue; btnEntladungStart.ImageIndex = entladung.EntladeStart.HasValue ? 0 : -1; btnEntladungStart.Enabled = ((!entladung.EntladeStart.HasValue) | ((entladung.EntladeStart.HasValue && !entladung.EntladeEnde.HasValue))); btnEntladungEnde.ImageIndex = entladung.EntladeEnde.HasValue ? 0 : -1; btnEntladungEnde.Enabled = (entladung.EntladeStart.HasValue && !entladung.AusfahrtLKW.HasValue);
Man beachte die Zeilenzahlen. Die obere Variante hat genau die 4 fache Anzahl Code.
Obwohl, schön aussehen tun die vielen If’s und Else schon 🙂
Und da der untere Code viel kürzer und einfacher ist, braucht man auch nichts mehr mit Region umklammern.
Ich wuerde hier noch einen Schritt weiter gehen und ein paar Zeilen mehr mit verbesserter Uebersichtlichkeit eintauschen:
Korrekterweise sollten die bool Variablen auch so spaet wie moeglich deklariert werden. Das wuerde dann aber dazu fuehren, dass man versucht ist die einzelnen Bloecke weiter in Unterfunktionen zu packen. 🙂
Auch Bingo 🙂
Danke für die Anregung.
HP