Top 6 Code Smells in MVC 1


Verbreitet die Message!

Das Model View Controller (MVC) Modell ist in der Web-Entwicklung extrem verbreitet. In dem Paper “Code smells for Model-View-Controller architectures” beschreiben die Autoren sechs “Code-Smells” – ein von Martin Fowler geprägter Begriff, der soviel wie Code-Fehler bedeutet – die durch Interviews und Umfragen mit Entwicklern gefunden wurden.

1. Fat Repository

Repositories wickeln die Kommunikation mit dem Persistenz-Mechanismus und damit mit der Datenbank im Model-Layer in MVC ab. Dabei sollte ein Repository auch nur eins zu eins mit einem Entity interagieren. Ist dies nicht der Fall spricht man vom “Fat Repository”.
Ein Hauptmotivation ist die Lesbarkeit und damit die Wartbarkeit, die stark darunter leidet wenn ein Repository mehrere Entities auf einmal bedient. Wenn es zu dem bedienten Entitiy auch noch ein eigenes Repository gibt, ist die Verwirrung enorm.

Man denke immer an den armen Entwickler, der bei der Suche eines Fehlers über den Controller zum Service kommt, dort Code liest und erwartet, dass die Daten für ein ‘EntityXyz’ auch aus ‘EntityXyzRepository’ stammen. Jetzt beginnt der arme Kerl vielleicht im ‘EntityXyzRepository’ den Fehler zu suchen und stellt nach einiger Zeit fest, dass ein Teil der Daten auch noch aus ‘FatRepo’ stammt.

Aber auch das Testen, bei dem man sich oft des Mechanismus der Dependency Injection bedient kann dadurch behindert sein, dass man eventuell ein Repository mocken möchte, das plötzlich ein nicht betrachtetes Entitity bedient.

Refactoring

Es ist relativ offensichtlich, dass sobald ein Repository mehrere Entities bedient, die entsprechenden Methoden und Funktionalitäten in das richtige Repository verschoben werden muss (oder dieses erstellt werden muss). Wie bei allen Code Smells ist Disziplin gefragt und Clean-Coder-Codex. Quick & Dirty ist einfach nur Dirty und niemals Quick.

Häufigkeit und Schwere

Das Fat Repository war mit 20,5% überraschenderweise am häufigsten vertreten und wurde von den Developern als sehr negativ bewertet. Man könnte meinen dieser Smell sei besonders leicht zu erkennen und und würde aufgrund seiner Offensichtlichkeit vermieden, doch dies ließe sich vielleicht mit der Erkenntnis der Autoren erklären, dass sich Code Smells generell lange im Code halten. Verwunderlich ist dies auch, da dieser Code Smell verhältnismäßig einfach zu refactoren ist.

2. Promiscuous Controller

Controller sollten immer kleine Klassen sein, die zusammenhängende Schnittstellen/Endpunkte zum Client bieten. Bietet ein und derselbe Controller zu viele, unterschiedliche Enpunkte an spricht man vom Promiscuous Controller. Gemeint ist damit nicht eine hohe Komplexität, sondern das bedienen zu vieler erreichbarer Routen, wie bspw. das Anzeigen, Updaten, Erstellen und Löschen eines Domain-Objektes, wie aber auch dessen zahlreiche Statusveränderungen, die von der Userseite von verschiedensten Stellen kommen können.

Die Wartbarkeit leidet stark, wenn wie auch beim Fat Repository erklärt, die Kohäsion/Zusammengehörigkeit fehlt. Die “Routen” die nach außen hin verfolgt werden sind dann schwer nachzuvollziehen, zu verstehen und damit zu debuggen.

Auch das Testen wird erschwert, da die Funktionalität der Controller durch Integrationstests getestet werden müssen. Diese werden durch zuviele Endpunkte zu aufwendig und damit wahrscheinlich deaktiviert.

Refactoring

Es führt nichts daran vorbei, den Controller möglichst früh in mehrere Controller aufzubrechen, sollte man merken, dass er zu lang, bzw. zu viele unterschiedliche Funktionalitäten (Koharänz) bedient. Das ist ein aufwendiger Prozess, da dies nicht nur heißen kann die dadurch entstehenden Controller wieder zu refactoren, sondern dass auch immer wieder kleinere Fehler durch das Refactoring selbst entstehen können. Hat man keine Tests, fehlt die Sicherheit, hat man Tests muss man diese ebenfalls refactoren.

Dennoch lohnt sich der Prozess aufgrund des Effekts von steigenden Kosten bei steigenden technischen Schulden!

Häufigkeit und Schwere

Die Studie zeigt, dass Developer den Promiscuous Controller als relativ negativ bewertet haben und in knapp 12% der untersuchten Controller dieses Pattern aufweisen.

3. Brain Controller

Managt ein Controller einen komplexen Kontrollfluss bezeichnet man ihn als Brain Controller. Controller sollten dumm sein und lediglich Endpunkte an den User bieten. Komplexe Funktionalität/Business Logik sollte in Services, Datenbank-Interaktionen in ein Repository ausgelagert werden.

Komplexe Funktionalität im Brain Controller zu implementieren, bedeutet das M in MVC zu streichen. Auch wenn der Brain Controller nur wenige Endpunkte zum User bietet (und damit kein Promiscuous Controller ist) führt hohe Komplexität zu geringerer Wartbarkeit.

Man stelle sich auch hier wieder den Developer vor, der einen Logikfehler beispielsweise in einer Datenserialisierung sucht. Er sollte diesen niemals im Controller suchen müssen – eine Layer die für die Bereitstellung von Endpunkten und Verhandlung von Model und View verantwortlich ist. Außerdem ist es in unserem Szenario beim Brain Controller wahrscheinlich, dass ein Teil der eigentlich koharänten Datenserialisierung im Controller und ein Teil in einem Service abgewickelt wird, was wirklich hässlich ist.

Bei den Tests könnte es auftreten, das Controller prinzipiell nicht Unit-Tests (sondern Integrationstests) unterliegen, da angenommen wird, dass diese dumm sind. So geht Testabdeckung verloren, bzw. schlagen eventuell Integrationstest wegen komplexer Seiteneffekte im Brain Controller fehl.

Refactoring

Wie eingangs erwähnt muss Code der Business-Logik in Services, Entities oder Komponenten ausgelagert werden. Datenbank-Interaktionen gehören in ein Repository.

Häufigkeit und Schwere

Der Brain Controller kam mit ca. 7,5 % ziemlich häufig in den untersuchten Repositories vor und wurde gleichzeitig äußerst negativ bewertet. Im Anbetracht der Tatsache, dass die Wartbarkeit sehr darunter leidet, sollte also hier besonders darauf geachtet werden, diesen Fehler zu vermeiden oder früh zu refactoren.

4. Brain Repository

Wie bereits erwähnt dient ein Repository lediglich der Bedienung des Persistenz-Mechanismus. Wenn ein Repository jedoch einen Teil Business-Logik enthält ist dies ein Bruch des MVC-Patterns. Aber auch komplexe SQL-Abfragen können schon als versteckte Abhandlung von Business-Logik verstanden werden.

Gerade letzteres ist absolut kritisch, da um Code zu verstehen (und damit zu warten), der .NET Spezialist eventuell SQL-Spezialist sein muss.
Auch das Testframework testet gemeinhin die Sprache in der der Controller geschrieben ist – und das ist nicht SQL. Es leidet Wartbarkeit und Testbarkeit.

Ein Repository sollte Daten speichern, holen und verändern (suchen & sortieren) können. Nicht mehr. Zu erkennen ist das Brain Repository auch an der Verwendung von Kontrollstrukturen wie if oder for – das Repo soll nur Daten holen, suchen, sortieren und filtern – nicht komplexe Fälle unterscheiden.

Refactoring

Es ist leicht das Brain Repository zu vermeiden, indem man komplexe Logik in Services oder Komponenten verschiebt und SQL-Abfragen in private Methoden aufsplittet. Nur wenn es sehr gute Argumente für Performance-Verluste gibt, sollte man allgemein von “dummen” Methoden abweichen.

Häufigkeit und Schwere

7% der untersuchten Klassen waren Brain Repositories, diese wurden aber im Mittel zwischen mittel und und kritischer bewertet. Die Behebung des Fehlers ist unschön und könnte Seiteneffekte haben, deshalb ist der Brain Controller unbedingt in der Entstehung zu vermeiden.

5. Laborious Repository Method

Eine Methode sollte laut “Uncle Bob” immer nur eine einzige Sache erledigen. Wenn dann im Repository mehrere Abfragen/Requests an die Datenbank geschickt werden widerspricht diese “Laborious Repository Method” (Mühselige Repository Methode) diesem Prinzip. Es schädigt wieder der Kohäsion und Verständlichkeit des Codes und damit stark der Wartbarkeit.

Refactoring

Die einfachste Vorgehensweise ist es, die Abfragen erst einmal in eigene Methoden auszulagern.  Es ist außerdem fraglich, ob die Notwendigkeit mehrerer Abfragen wirklich gegeben ist und wenn ja, ob das Datenbankmodell nicht verbessert werden muss. Natürlich sollte nicht aus Vermeidung dieses Fehlers das Repository wie oben beschrieben zum Brain Repo werden, indem man zwei Abfragen zu einer komplexen macht.

Häufigkeit und Schwere

6,6% der Untersuchten Smells waren LRM-Smells. Allgemein wurde dieser Fehler als nicht allzu dramatisch bewertet und vom Großteil der Befragten zwischen 0-3 bewertet. Dennoch sollte man sich von der Lässigkeit mancher Entwickler nicht mitreissen lassen und diesen Fehler beheben – zumal dies nicht sehr komplex sein sollte.

6. Meddling Service

In Services wird komplexe Busines-Logik und komplexer Kontroll-Fluss zwischen anderen Klassen bereitgestellt. Dafür benötigte Daten sollten aus dem Repository kommen. Tatsächlich kommt es trotz Repositories vor, dass Daten mittels SQL-Befehlen direkt in den Service geladen werden – man spricht vom Meddling Service (Aufdringlicher Dienst) . Auch hier wird wieder das Prinzip der Kapselung verletzt und erschwert damit die ohne erkennbaren Nutzen Wartbarkeit enorm. Dazu gibt es nichts weiter zu sagen als: schlechter Stil.

Refactoring

Es ist sehr einfach: Verlagere den SQL-Code in ein Repository, am besten vor deinem Commit und spätestens vorm nächsten Review..

3,9% der untersuchten Smells wiesen diesen ärgerlichen, sehr offensichtlichen und einfach zu behebenden Fehler auf – auch die befragten Entwickler waren in ihrer Einschätzung nicht begeistert und bewerteten den Fehler als mittelschwer.

Fazit

Das Wissen um die häufigsten und von unseren Kollegen und Peers am kritischsten wahrgenommen Fehler in MVC Architektur kann dazu benutzt werden, diese früher zu erkennen und damit Geld, Zeit und Nerven zu sparen.

Durch den erhöhten Aufwand beim Refactoring des Brain Controllers und Brain Repositories sollte dieser früh vermieden werden.

Bei allen genannten Smells ist insbesondere wichtig zu erkennen, dass das nötige Refactoring immer aufwendiger wird, umso mehr an einem Smell “herumprogrammiert” wird. Ein Issue schnell zu fixen ist wie Schmerztabletten bei Karies zu schlucken – es führt kein Weg am Zahnarzt vorbei, die Frage ist nur bohren oder Wurzelbehandlung?

Im Projekt heißt es oft: “Dafür haben wir jetzt keine Zeit (mehr)”. Die Antwort muss ein klares “Wir müssen Zeit dafür haben. Es ist unsere Pflicht!” – dazu benötigt ein Entwickler die Integrität die in Clean Coder besprochen wird und die nötige Disziplin und das Verantwortungsbewusstsein, den Code Smell zu beseitigen.

Was naiv klingt wird ganz rational und wirtschaftlich begründbar, wenn man sich Kosten technischer Schulden im Gegensatz zu Investition in Code Qualität anschaut. Es lohnt sich, auf die Vermeidung dieser Fehler zu achten, für bessere Software, glücklichere Kollegen & Kunden und bessere Projekte.


1: Code smells for Model-View-Controller architectures: Aniche, M., Bavota, G., Treude, C. et al. Empir Software Eng (2017). https://doi.org/10.1007/s10664-017-9540-2

 

Verbreitet die Message!

Schreiben Sie einen Kommentar

Ein Gedanke zu “Top 6 Code Smells in MVC