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

Question

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

public class Impersonator 
{ 
    private ImpersonationContext _context; 

    public void Impersonate() 
    { 
     ... 
     _context = GetContext(); 
     ... 
    } 

    public void UndoImpersonation() 
    { 
     if (_context != null) 
      _someDepend.Undo(); 
    } 
} 

was bedeutet, dass zu Test UndoImpersonation, ich brauche es einzurichten durch Impersonate Aufruf (Impersonate hat bereits mehrere Komponententests sein Verhalten zu überprüfen). Das riecht mir schlecht, aber in einem gewissen Sinn macht es Sinn, aus der Sicht des Codes, der in dieser Klasse ruft:

public void ExerciseClassToTest(Impersonator c) 
{ 
    try 
    { 
     if (NeedImpersonation()) 
     { 
      c.Impersonate(); 
     } 
     ... 
    } 
    finally 
    { 
     c.UndoImpersonation(); 
    } 
} 

Ich würde das nicht ausgearbeitet haben, wenn ich nicht versuchen, einen schreiben Unit-Test für UndoImpersonation und fand mich selbst den Test durch Aufruf der anderen öffentlichen Methode einrichten. Also, ist das ein schlechter Geruch und wenn ja, wie kann ich damit umgehen?

Answer 1

Code-Geruch muss einer der vage Begriffe sein, die ich jemals in der Programmierwelt begegnet bin. Für eine Gruppe von Leuten, die sich auf technische Prinzipien stützen, steht sie direkt da oben in Bezug auf unermesslichen Unsinn und etwa so nutzlos wie LOCs pro Tag für die Effizienz von Programmierern.

Wie auch immer, dass mein rant ist, vielen Dank für Zuhören :-)

Um Ihre Frage zu beantworten, ich glaube nicht, dass dies ist ein Problem. 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 das sein, was passiert, wenn man es ohne erste Einstellung aufrufen die Voraussetzungen - es entweder ausfallen sollte anmutig oder seine eigene Voraussetzung, wenn der Anrufer nicht zu tun einrichten gestört hat .

Answer 2

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

Initialisierung Felder in einer Instanz-Methode 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 single step construction in Ward Cunninghams Wiki.

Der Grund für die Initialisierung von Feldern in einer Instanzmethode ist in erster Linie, 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 diese Art der Kopplung kann unvermeidlich sein (zB wenn eine Methode eine Ressource wie ein Datei-Handle erwirbt, und eine andere Methode ist erforderlich, es zu veröffentlichen). Aber auch das sollte innerhalb einer Methode, wenn möglich behandelt werden.

Was ist mit Ihrem Fall gilt nur schwer ohne mehr Kontext zu erzählen.

Answer 3

Sie veränderbare Objekte nicht berücksichtigen Vorausgesetzt ein Code-Geruch von sich selbst, ein Objekt in den für einen Test benötigten Zustand zu versetzen, ist einfach Teil des Setups für diesen Test.

Answer 4

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

immer Sie wollen sehr vorsichtig sein, wenn dies zu tun, dass das Muster etwas leichter zu verstehen ist - zum Beispiel denke ich, die meisten Nutzer akzeptieren diese Art von Verhalten für etwas Transaktions-, aber vielleicht überrascht sein, wenn sie DoStuff() und TheMethodIWantToTest() begegnen (was auch immer sie bin wirklich genannt).

Es ist normalerweise die beste Vorgehensweise, eine Eigenschaft zu 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 nie 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 einer Eigenschaft sein.

Answer 5

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

Die Höhe der Verkettungs hängt von der Gestaltung des Objekts - Sie können entweder Gabel oder Kaskade.

• Forking: classToTest1.SomeDependency.DoSomething()
• Kaskadierung: classToTest1.DoSomething() (die intern würde SomeDependency.DoSomething nennen)

Aber wie andere schon erwähnt haben, auf jeden Fall in den Konstruktor Ihren Zustand Initialisierung halten was von dem, was ich sagen kann, wird wahrscheinlich Ihr Problem lösen.

Answer 6

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 eine Impersonator zu haben, die in verschiedenen Persona-Zuständen sein kann, warum nicht eine unveränderliche Persona? Wenn Sie eine andere Persona benötigen, erstellen Sie einfach eine neue, anstatt den Status eines vorhandenen Objekts zu ändern. Wenn Sie danach eine Reinigung vornehmen müssen, machen Sie Persona Einweg. Sie können die Impersonator Klasse als Fabrik halten:

using (var persona = impersonator.createPersona(...)) 
{ 
    // do something with the persona 
}