Ist es ein Code-Geruch für eine Methode, um von einer anderen abhängig zu sein?

7

Ich refactoring eine Klasse, so dass der Code testbar ist (mit NUnit und RhinoMocks als Test - und Isolierungsframeworks) und festgestellt habe, dass ich mit einer Methode gefunden habe, ist abhängig von einer anderen (dh es hängt von etwas ab, das von erstellt wird diese andere Methode). Etwas wie das Folgende:

%Vor%

Das bedeutet, dass ich UndoImpersonation testen muss, indem ich Impersonate aufruft (Impersonate hat bereits mehrere Unit-Tests, um sein Verhalten zu überprüfen). Das riecht schlecht für mich, aber in gewisser Hinsicht macht es Sinn aus der Sicht des Codes, der in diese Klasse einreiht:

%Vor%

Ich hätte das nicht herausgefunden, wenn ich nicht versucht hätte, einen Komponententest für UndoImpersonation zu schreiben. Ich musste den Test durch Aufruf der anderen öffentlichen Methode einrichten. Also, ist das ein schlechter Geruch und wenn ja, wie kann ich es umgehen?

    
jpoh 29.07.2009, 08:46
quelle

6 Antworten

9

Code-Geruch muss einer der vage Begriffe sein, die ich je in der Programmierwelt kennengelernt habe. Für eine Gruppe von Leuten, die sich auf technische Prinzipien stützt, rangiert sie direkt in Bezug auf unermesslichen Unsinn und etwa so nutzlos wie LOCs pro Tag für Programmierer Effizienz.

Wie auch immer, das ist meine Tirade, danke fürs Zuhören: -)

Um Ihre spezifische Frage zu beantworten, glaube ich nicht, dass ein Problem ist. Wenn Sie etwas mit Vorbedingungen testen, müssen Sie sicherstellen, dass die Vorbedingungen für den gegebenen Testfall zuerst eingerichtet wurden.

Einer der Tests sollte sein, was passiert, wenn Sie ohne aufrufen, bevor Sie die Vorbedingungen festlegen - er sollte entweder ordnungsgemäß fehlschlagen oder seine eigene Vorbedingung einrichten, wenn der Aufrufer dies nicht getan hat hat sich darum gekümmert.

    
paxdiablo 29.07.2009, 08:58
quelle
7

Nun, es gibt ein bisschen zu wenig Kontext zu sagen, es sieht so aus als ob _someDepend im Konstruktor initialisiert werden sollte.

Das Initialisieren von Feldern in einer Instanzmethode ist ein großes NEIN für mich. Eine Klasse sollte vollständig benutzbar sein (d. H. Alle Methoden funktionieren), sobald sie konstruiert ist; Daher sollten die Konstruktoren alle Instanzvariablen initialisieren. Siehe z.B. die Seite auf Einzelschritt-Konstruktion in Ward Cunninghams Wiki.

Der Grund für die Initialisierung von Feldern in einer Instanzmethode ist hauptsächlich, dass sie eine implizite Reihenfolge auferlegt, wie Methoden aufgerufen werden können. In Ihrem Fall wird TheMethodIWantToTest verschiedene Dinge tun, je nachdem, ob DoStuff zuerst aufgerufen wurde. Dies ist in der Regel nicht etwas, was ein Benutzer Ihrer Klasse erwarten würde, also ist es schlecht: - (.

Das heißt, manchmal ist diese Art von Kopplung unvermeidlich (z.B. wenn eine Methode eine Ressource wie eine Dateikennung anfordert und eine andere Methode benötigt wird, um sie freizugeben). Aber selbst das sollte wenn möglich innerhalb einer Methode behandelt werden.

Was für Ihren Fall zutrifft, ist ohne mehr Kontext schwer zu sagen.

    
sleske 29.07.2009 08:54
quelle
2

Wenn Sie veränderbare Objekte nicht für einen Code-Geruch halten, müssen Sie ein Objekt in den für einen Test erforderlichen Zustand versetzen. Dies ist nur ein Teil des Setups für diesen Test.

    
Steve Gilham 29.07.2009 08:56
quelle
2

Dies ist oft unvermeidlich, zum Beispiel wenn Sie mit entfernten Verbindungen arbeiten - Sie müssen Open() aufrufen, bevor Sie Close() aufrufen können, und Sie wollen nicht, dass Open() automatisch im Konstruktor auftritt.

Allerdings solltest du dabei sehr vorsichtig sein, dass das Muster etwas ist, was man leicht versteht - ich denke zum Beispiel, dass die meisten Benutzer diese Art von Verhalten für alles Transaktionale akzeptieren, aber vielleicht überrascht werden, wenn sie auf DoStuff() und TheMethodIWantToTest() treffen. (wie auch immer sie wirklich genannt werden).

Normalerweise ist es am besten, wenn Sie eine Eigenschaft haben, die den aktuellen Zustand darstellt. Sehen Sie sich Fern- oder DB-Verbindungen an, um ein Beispiel für ein konsistent verstandenes Design zu erhalten.

Das große Nein ist, dass dies für Immobilien jemals passieren wird. Eigenschaften sollten sich nie darum kümmern, in welcher Reihenfolge sie aufgerufen werden. Wenn Sie einen einfachen Wert haben, der von der Reihenfolge der Methoden abhängt, sollte es eine parameterlose Methode anstelle eines property-get sein.

    
Keith 29.07.2009 09:15
quelle
2

Ja, ich denke, in diesem Fall gibt es einen Code-Geruch. Nicht wegen Abhängigkeiten zwischen Methoden, sondern wegen der vagen Identität des Objekts. Anstatt ein Impersonator zu haben, das in verschiedenen Persona-Zuständen sein kann, warum nicht ein unveränderliches Persona ?

haben

Wenn Sie eine andere Persona benötigen, erstellen Sie einfach eine neue, anstatt den Status eines vorhandenen Objekts zu ändern. Wenn Sie danach etwas aufräumen müssen, machen Sie Persona disposable. Sie können die Klasse Impersonator als Factory beibehalten:

%Vor%     
Wim Coenen 29.07.2009 14:11
quelle
1

Um den Titel zu beantworten: Methoden zu haben, die sich gegenseitig anrufen (Verkettung), ist in der objektorientierten Programmierung unvermeidlich, daher ist es meiner Ansicht nach nicht falsch, eine Methode zu testen, die eine andere aufruft. Ein Komponententest kann schließlich eine Klasse sein, es ist eine "Einheit", die Sie testen.

Die Ebene der Verkettung hängt vom Design Ihres Objekts ab - Sie können entweder fork oder cascade .

  • Abzweigung: %Code%
  • Kaskadierung : classToTest1.SomeDependency.DoSomething() (was intern SomeDependency.DoSomething aufrufen würde)

Aber wie andere bereits erwähnt haben, sollten Sie Ihre Zustandsinitialisierung im Konstruktor beibehalten, was, wie ich feststellen kann, wahrscheinlich Ihr Problem lösen wird.

    
Chris S 29.07.2009 09:33
quelle

Tags und Links