Clean Code: Konsequenzen kurzer Methoden mit wenigen Parametern

Kürzlich bin ich bei einer Codeüberprüfung auf Code gestoßen, der von einem neuen Kollegen geschrieben wurde und ein Muster mit einem Geruch enthält. Ich vermute, dass die Entscheidungen meines Kollegen auf Regeln basieren, die vom berühmten Clean Code-Buch (und vielleicht auch von anderen ähnlichen Büchern) vorgeschlagen wurden.

Nach meinem Verständnis ist der Klassenkonstruktor vollständig für die Erstellung eines gültigen Objekts verantwortlich und seine Hauptaufgabe ist die Zuweisung der (privaten) Eigenschaften eines Objekts. Es kann natürlich vorkommen, dass optionale Eigenschaftswerte von anderen Methoden als dem Klassenkonstruktor festgelegt werden. Solche Situationen sind jedoch eher selten (obwohl dies nicht unbedingt falsch ist, vorausgesetzt, der Rest der Klasse berücksichtigt die Optionalität einer solchen Eigenschaft). Dies ist wichtig, um sicherzustellen, dass sich das Objekt immer in einem gültigen Zustand befindet.

In dem Code, auf den ich gestoßen bin, werden die meisten Eigenschaftswerte jedoch durch andere Methoden als den Konstruktor festgelegt. Werte, die aus Berechnungen resultieren, werden Eigenschaften zugewiesen, die innerhalb mehrerer privater Methoden in der Klasse verwendet werden sollen. Der Autor verwendet scheinbar Klasseneigenschaften, als wären sie globale Variablen, auf die in der gesamten Klasse zugegriffen werden sollte, anstatt diese Werte für die Funktionen zu parametrisieren, die sie benötigen. Außerdem sollten die Methoden der Klasse in einer bestimmten Reihenfolge aufgerufen werden, da die Klasse sonst nicht viel macht.

Ich vermute, dass dieser Code von dem Rat inspiriert wurde, Methoden kurz zu halten (<= 5 Codezeilen), um große Parameterlisten (<3 Parameter) zu vermeiden und dass Konstruktoren keine Arbeit leisten dürfen (z. B. eine Art Berechnung durchführen) Dies ist für die Gültigkeit des Objekts von wesentlicher Bedeutung.

Jetzt könnte ich natürlich gegen dieses Muster vorgehen, wenn ich nachweisen kann, dass alle möglichen undefinierten Fehler auftreten, wenn Methoden nicht in einer bestimmten Reihenfolge aufgerufen werden. Ich gehe jedoch davon aus, dass als Reaktion darauf Überprüfungen hinzugefügt werden, die bestätigen, dass Eigenschaften festgelegt werden müssen, sobald Methoden aufgerufen werden, für die diese Eigenschaften festgelegt werden müssen.

Ich würde jedoch eher vorschlagen, den Code vollständig zu ändern, sodass die Klasse zu einer Blaupause für ein tatsächliches Objekt wird und nicht zu einer Reihe von Methoden, die (prozedural) in einer bestimmten Reihenfolge aufgerufen werden sollten.

Ich habe das Gefühl, dass der Code, dem ich begegnet bin, riecht. Tatsächlich glaube ich, dass es eine ziemlich klare Unterscheidung gibt, wann ein Wert in einer Klasseneigenschaft gespeichert und wann er in einem Parameter für eine andere Methode abgelegt werden muss - ich glaube nicht wirklich, dass sie Alternativen zueinander sein können . Ich suche die Wörter für diese Unterscheidung.

Als jemand, der Clean Code mehrmals gelesen und die Clean Coders-Reihe angesehen hat und oft andere Leute darin unterrichtet und coacht, saubereren Code zu schreiben, kann ich in der Tat dafür bürgen, dass Ihre Beobachtungen richtig sind - die Metriken, auf die Sie hinweisen, werden alle im Buch erwähnt .

In dem Buch werden jedoch noch weitere Punkte angesprochen, die neben den von Ihnen genannten Leitlinien gelten sollten. Diese wurden im Code, mit dem Sie sich befassen, anscheinend ignoriert. Möglicherweise ist dies darauf zurückzuführen, dass sich Ihr Kollege noch in der Lernphase befindet. In diesem Fall ist es wichtig, sich daran zu erinnern, dass er es mit gutem Willen tut, lernt und versucht , auch wenn es notwendig ist, auf die Gerüche seines Codes hinzuweisen um besseren Code zu schreiben.

Clean Code schlägt vor, dass Methoden kurz und mit möglichst wenigen Argumenten sein sollten. In Übereinstimmung mit diesen Leitlinien wird jedoch vorgeschlagen, die S OLID-Grundsätze einzuhalten, den Zusammenhalt zu stärken und die Kopplung zu verringern .

Das S in SOLID steht für Single Responsibility Principle, das besagt, dass ein Objekt nur für eine Sache verantwortlich sein soll. "Thing" ist kein sehr präziser Begriff, daher variieren die Beschreibungen dieses Prinzips stark. Onkel Bob, der Autor von Clean Code, ist jedoch auch derjenige, der dieses Prinzip geprägt hat. Er sagt weiter, was er mit Gründen meint , sich hier und hier zu ändern(Eine längere Erklärung wäre hier zu viel). Wenn dieses Prinzip auf die Klasse angewendet wird, mit der Sie sich befassen, ist es sehr wahrscheinlich, dass die Teile, die sich mit Berechnungen befassen, von denen, die sich mit dem Haltezustand befassen, getrennt werden, indem die Klasse in zwei oder mehr geteilt wird, je nachdem, wie viele Gründe vorliegen diese Berechnungen zu ändern haben.

Außerdem sollten Clean- Klassen kohäsiv sein , was bedeutet, dass die meisten Methoden die meisten Attribute verwenden. Als solche ist eine Klasse mit maximalem Zusammenhalt eine Klasse, in der alle Methoden alle Attribute verwenden. In einer grafischen App können Sie beispielsweise eine VectorKlasse mit Attributen haben Point aund Point b, wo die einzigen Methoden sind scaleBy(double factor)und printTo(Canvas canvas), beide mit beiden Attributen arbeiten. Im Gegensatz dazu ist eine Klasse mit minimalem Zusammenhalt eine Klasse, bei der jedes Attribut nur in einer Methode verwendet wird und von jeder Methode nie mehr als ein Attribut verwendet wird. Im Durchschnitt einer Klasse präsentiert nichtbindigem „Gruppen“ von zusammenhängenden Teilen - dh einiger Methoden verwenden Attribute a, bund c, während des Rest Nutzung cundd - Das heißt, wenn wir die Klasse in zwei Teile aufteilen, erhalten wir zwei zusammenhängende Objekte.

Schließlich sollten Clean- Klassen die Kopplung so weit wie möglich reduzieren . Obwohl es viele Arten von Kopplungen gibt, die hier erörtert werden sollten, scheint der vorliegende Code hauptsächlich an einer zeitlichen Kopplung zu leiden , bei der die Methoden des Objekts, wie Sie bereits betont haben, nur dann erwartungsgemäß funktionieren, wenn sie in der richtigen Reihenfolge aufgerufen werden. Und wie bei den beiden oben genannten Richtlinien besteht die Lösung normalerweise darin, die Klasse in zwei oder mehr zusammenhängende Objekte aufzuteilen . Die Aufteilungsstrategie beinhaltet in diesem Fall normalerweise Muster wie Builder oder Factory und in sehr komplexen Fällen State-Machines.

Die TL; DR: Die Clean Code-Richtlinien, die Ihr Kollege befolgt hat, sind gut, aber nur dann, wenn Sie auch die verbleibenden Prinzipien, Praktiken und Muster befolgen, die im Buch erwähnt werden. Die bereinigte Version der "Klasse", die Sie sehen, wird in mehrere Klassen aufgeteilt, jede mit einer Verantwortung, zusammenhängenden Methoden und ohne zeitliche Kopplung. In diesem Kontext sind kleine Methoden und wenig bis gar keine Argumente sinnvoll.

Nach meinem Verständnis ist der Klassenkonstruktor vollständig für die Erstellung eines gültigen Objekts verantwortlich und seine Hauptaufgabe ist die Zuweisung der (privaten) Eigenschaften eines Objekts.

Es ist normalerweise dafür verantwortlich, das Objekt in einen anfänglich gültigen Zustand zu versetzen, ja; Andere Eigenschaften oder Methoden können dann den Status in einen anderen gültigen Status ändern.

In dem Code, auf den ich gestoßen bin, werden die meisten Eigenschaftswerte jedoch durch andere Methoden als den Konstruktor festgelegt. Werte, die aus Berechnungen resultieren, werden Eigenschaften zugewiesen, die innerhalb mehrerer privater Methoden in der Klasse verwendet werden sollen. Der Autor verwendet scheinbar Klasseneigenschaften, als wären sie globale Variablen, auf die in der gesamten Klasse zugegriffen werden sollte, anstatt diese Werte für die Funktionen zu parametrisieren, die sie benötigen. Außerdem sollten die Methoden der Klasse in einer bestimmten Reihenfolge aufgerufen werden, da die Klasse sonst nicht viel macht.

Neben den Problemen mit der Lesbarkeit und Wartbarkeit, auf die Sie anspielen, scheint es, dass in der Klasse selbst mehrere Phasen des Datenflusses / der Datenumwandlung stattfinden, was darauf hindeuten kann, dass die Klasse gegen das Prinzip der einfachen Verantwortung verstößt.

vermuten, dass dieser Code von dem Rat inspiriert wurde, Methoden kurz zu halten (<= 5 Codezeilen), um große Parameterlisten (<3 Parameter) zu vermeiden und dass Konstruktoren keine Arbeit leisten dürfen (z. B. eine solche Berechnung durchführen) ist wesentlich für die Gültigkeit des Objekts).

Das Befolgen einiger Codierungsrichtlinien, während andere ignoriert werden, führt häufig zu dummem Code. Wenn wir zum Beispiel vermeiden möchten, dass ein Konstrukteur Arbeiten ausführt, ist es normalerweise sinnvoll, die Arbeiten vor dem Bau auszuführen und das Ergebnis dieser Arbeiten an den Konstrukteur weiterzugeben. (Ein Argument für diesen Ansatz könnte sein, dass Sie es vermeiden, Ihrer Klasse zwei Verantwortlichkeiten zu übertragen: die Arbeit bei der Initialisierung und ihre 'Hauptaufgabe', was auch immer das ist.)

Ich habe die Erfahrung gemacht, dass ich Klassen und Methoden nur selten klein halten muss, sondern dass dies ganz natürlich aus einer einzelnen Verantwortung resultiert.

Ich würde jedoch eher vorschlagen, den Code vollständig zu ändern, sodass die Klasse zu einer Blaupause für ein tatsächliches Objekt wird und nicht zu einer Reihe von Methoden, die (prozedural) in einer bestimmten Reihenfolge aufgerufen werden sollten.

Sie hätten wahrscheinlich Recht damit. Es ist nichts Falsches daran, einfachen Verfahrenscode zu schreiben. Es gibt ein Problem, das OO-Paradigma zu missbrauchen, um verschleierten prozeduralen Code zu schreiben.

Ich glaube, es gibt eine ziemlich klare Unterscheidung, wann ein Wert in einer Klasseneigenschaft gespeichert und wann er in einen Parameter für eine andere Methode eingefügt werden muss - ich glaube nicht wirklich, dass sie Alternativen zueinander sein können. Ich suche die Wörter für diese Unterscheidung.

Normalerweise sollten Sie keinen Wert in ein Feld einfügen, um ihn von einer Methode zur anderen zu übertragen. Ein Wert in einem Feld sollte ein aussagekräftiger Bestandteil des Zustands eines Objekts zu einem bestimmten Zeitpunkt sein. (Ich kann mir einige gültige Ausnahmen vorstellen, aber keine, bei denen solche Methoden öffentlich sind oder bei denen eine Ordnungsabhängigkeit besteht, die ein Benutzer der Klasse kennen sollte.)

Sie schimpfen hier höchstwahrscheinlich gegen das falsche Muster. Kleine Funktionen und geringe Arität sind für sich genommen selten problematisch. Das eigentliche Problem hierbei ist die Kopplung, die eine Abhängigkeit der Funktionen von der Reihenfolge verursacht. Suchen Sie daher nach Möglichkeiten, um dies zu beheben, ohne die Vorteile der kleinen Funktionen zu vernachlässigen.

Code spricht mehr als Worte. Leute erhalten diese Art von Korrekturen viel besser, wenn Sie tatsächlich einen Teil des Refactorings durchführen und die Verbesserung zeigen können, vielleicht als Paarprogrammierübung. Wenn ich das mache, finde ich es oft schwieriger als ich dachte, das Design richtig zu machen und alle Kriterien in Einklang zu bringen.