Ist es eine schlechte Praxis, Code ausschließlich zu Testzwecken zu ändern?

Ich habe eine Debatte mit einem Programmiererkollegen darüber, ob es eine gute oder eine schlechte Praxis ist, ein funktionierendes Codestück nur so zu ändern, dass es testbar ist (z. B. durch Unit-Tests).

Meiner Meinung nach ist es in Ordnung, im Rahmen der Einhaltung guter objektorientierter und Software-Engineering-Praktiken (nicht "alles öffentlich machen" usw.).

Die Meinung meines Kollegen ist, dass es falsch ist, Code (der funktioniert) nur zu Testzwecken zu ändern.

Nur ein einfaches Beispiel: Stellen Sie sich diesen Code vor, der von einer Komponente (in C # geschrieben) verwendet wird:

public void DoSomethingOnAllTypes()
{
    var types = Assembly.GetExecutingAssembly().GetTypes();

    foreach (var currentType in types)
    {
        // do something with this type (e.g: read it's attributes, process, etc).
    }
}

Ich habe vorgeschlagen, dass dieser Code geändert werden kann, um eine andere Methode aufzurufen, die die eigentliche Arbeit erledigt:

public void DoSomething(Assembly asm)
{
    // not relying on Assembly.GetExecutingAssembly() anymore...
}

Bei dieser Methode wird ein Assembly-Objekt zur Bearbeitung benötigt, sodass Sie Ihre eigene Assembly übergeben können, um die Tests durchzuführen. Mein Kollege hielt das nicht für eine gute Praxis.

Was ist eine gute und gängige Praxis?

Das Ändern von Code, um ihn testbarer zu machen, bietet Vorteile, die über die Testbarkeit hinausgehen. Im Allgemeinen Code, der besser getestet werden kann

• Ist leichter zu pflegen,
• Ist einfacher zu überlegen,
• Ist lockerer gekoppelt und
• Hat ein besseres Gesamtdesign, architektonisch.

Es sind (scheinbar) gegensätzliche Kräfte im Spiel.

• Einerseits möchten Sie die Kapselung erzwingen
• Andererseits möchten Sie die Software testen können

Befürworter der Geheimhaltung aller "Implementierungsdetails" sind in der Regel von dem Wunsch motiviert, die Kapselung beizubehalten. Es ist jedoch eine missverstandene Herangehensweise an die Verkapselung , alles verschlossen und nicht verfügbar zu halten. Wenn es das ultimative Ziel wäre, alles nicht verfügbar zu machen, wäre der einzig wahre gekapselte Code folgender:

static void Main(string[] args)

Schlägt Ihr Kollege vor, dies zum einzigen Zugangspunkt in Ihrem Code zu machen? Sollte jeder andere Code für externe Anrufer unzugänglich sein?

Kaum. Was macht es dann in Ordnung, einige Methoden öffentlich zu machen? Ist es nicht am Ende eine subjektive Designentscheidung?

Nicht ganz. Was Programmierer auch auf unbewusster Ebene anleitet, ist wiederum das Konzept der Kapselung. Sie fühlen sich sicher, eine öffentliche Methode verfügbar zu machen, wenn sie ihre Invarianten ordnungsgemäß schützt .

Ich würde nicht eine private Methode verfügbar machen möchten , die nicht seine Invarianten nicht schützen, aber oft kann man es ändern , so dass es nicht seine Invarianten zu schützen, und dann an die Öffentlichkeit aussetzen (natürlich mit TDD, tun Sie es den anders herum).

Das Öffnen einer API für die Testbarkeit ist eine gute Sache , da Sie das Open / Closed-Prinzip wirklich anwenden .

Wenn Sie nur einen einzigen Aufrufer Ihrer API haben, wissen Sie nicht, wie flexibel Ihre API wirklich ist. Die Chancen stehen gut, dass es ziemlich unflexibel ist. Tests fungieren als zweiter Client und geben Ihnen wertvolles Feedback zur Flexibilität Ihrer API .

Wenn also Tests ergeben, dass Sie Ihre API öffnen sollten, tun Sie dies. Behalten Sie jedoch die Kapselung bei, indem Sie nicht die Komplexität verbergen, sondern die Komplexität auf ausfallsichere Weise offenlegen.

Sieht so aus, als sprichst du über Abhängigkeitsinjektion . Es ist wirklich üblich und IMO, ziemlich notwendig für die Testbarkeit.

Um die allgemeinere Frage zu beantworten, ob es eine gute Idee ist, Code zu modifizieren, um ihn nur testbar zu machen, stellen Sie sich das so vor: Code hat mehrere Verantwortlichkeiten, einschließlich a) ausgeführt zu werden, b) von Menschen gelesen zu werden und c) zu getestet werden. Alle drei sind wichtig, und wenn Ihr Code nicht alle drei Verantwortlichkeiten erfüllt, würde ich sagen, dass es kein sehr guter Code ist. Also abändern!

Es ist ein kleines Henne-Ei-Problem.

Einer der Hauptgründe für eine gute Testabdeckung Ihres Codes ist, dass Sie damit furchtlos umgestalten können. Sie befinden sich jedoch in einer Situation, in der Sie den Code umgestalten müssen, um eine gute Testabdeckung zu erhalten! Und Ihr Kollege hat Angst.

Ich sehe den Standpunkt Ihres Kollegen. Sie haben Code, der (vermutlich) funktioniert, und wenn Sie ihn - aus welchen Gründen auch immer - überarbeiten, besteht die Gefahr, dass Sie ihn beschädigen.

Wenn es sich jedoch um Code handelt, für den eine fortlaufende Wartung und Änderung erwartet wird, besteht dieses Risiko jedes Mal, wenn Sie daran arbeiten. Und Refactoring jetzt und immer einige Testabdeckung jetzt können Sie dieses Risiko nehmen, unter kontrollierten Bedingungen und den Code in eine bessere Form für zukünftige Modifikation erhalten.

Daher würde ich sagen, dass Sie, sofern diese bestimmte Codebasis nicht ziemlich statisch ist und in Zukunft voraussichtlich keine nennenswerten Arbeiten mehr ausführen werden, eine gute technische Praxis wünschen.

Natürlich, ob es sich um eine gute Geschäftspraxis handelt , ist eine ganze Reihe von Würmern.

Dies kann nur ein Unterschied in der Betonung von den anderen Antworten, aber ich würde sagen , dass der Code sollte nicht Refactoring streng die Testbarkeit zu verbessern. Testbarkeit ist für die Wartung von großer Bedeutung, aber Testbarkeit ist kein Selbstzweck. Daher würde ich ein solches Refactoring so lange aufschieben, bis Sie vorhersagen können, dass dieser Code gewartet werden muss, um ein geschäftliches Ende zu erreichen.

An dem Punkt , dass Sie feststellen , dass dieser Code eine Wartung erfordern, dass wäre ein guter Zeitpunkt für Testbarkeit Refactoring sein. Abhängig von Ihrem Geschäftsfall kann es eine gültige Annahme sein, dass der gesamte Code irgendwann gewartet werden muss. In diesem Fall verschwindet die Unterscheidung, die ich mit den anderen Antworten hier mache ( z. B. die Antwort von Jason Swett ).

Zusammenfassend lässt sich sagen, dass Testbarkeit allein (IMO) kein ausreichender Grund ist, eine Codebasis umzugestalten. Testbarkeit spielt eine wichtige Rolle bei der Ermöglichung der Wartung auf Codebasis, aber es ist eine Geschäftsanforderung, die Funktion Ihres Codes zu ändern, die Ihr Refactoring vorantreiben soll. Wenn es keine solche Geschäftsanforderung gibt, ist es wahrscheinlich besser, an etwas zu arbeiten, das Ihren Kunden wichtig ist.

(Neuer Code wird natürlich aktiv gepflegt, daher sollte er so geschrieben werden, dass er testbar ist.)

Ich denke, dein Kollege ist falsch.

Andere haben bereits die Gründe genannt, warum dies eine gute Sache ist, aber solange Sie die Erlaubnis dazu erhalten, sollten Sie in Ordnung sein.

Der Grund für diese Einschränkung ist, dass das Vornehmen von Codeänderungen zu Lasten des Codes geht, der erneut getestet werden muss. Je nachdem, was Sie tun, kann diese Testarbeit für sich genommen einen großen Aufwand bedeuten.

Es ist nicht unbedingt Ihre Aufgabe, die Entscheidung für ein Refactoring zu treffen, anstatt an neuen Funktionen zu arbeiten, die Ihrem Unternehmen / Kunden zugute kommen.

Ich habe Codeabdeckungstools im Rahmen von Komponententests verwendet, um zu überprüfen, ob alle Pfade durch den Code ausgeführt wurden. Als ziemlich guter Coder / Tester alleine bedecke ich normalerweise 80-90% der Codepfade.

Wenn ich die ungedeckten Pfade studiere und mir Mühe gebe, dann entdecke ich Fehler, wie zum Beispiel Fehlerfälle, die "niemals passieren". Wenn Sie also den Code ändern und auf Testabdeckung prüfen, erhalten Sie einen besseren Code.

Ihr Problem hier ist, dass Ihre Testwerkzeuge Mist sind. Sie sollten in der Lage sein, dieses Objekt zu verspotten und Ihre Testmethode aufzurufen, ohne es zu ändern - denn während dieses einfache Beispiel wirklich einfach und leicht zu ändern ist, passiert was, wenn Sie etwas viel Komplizierteres haben.

Viele Leute haben ihren Code geändert, um IoC-, DI- und schnittstellenbasierte Klassen einzuführen, um das Testen von Einheiten mithilfe der Tools zum Verspotten und Testen von Einheiten zu ermöglichen, für die diese Codeänderungen erforderlich sind. Ich denke nicht, dass sie eine gesunde Sache sind, nicht, wenn man sieht, dass Code, der ziemlich einfach und unkompliziert war, sich in ein Alptraum-Durcheinander komplexer Interaktionen verwandelt, getrieben von der Notwendigkeit, jede Klassenmethode vollständig von allem anderen zu entkoppeln . Und um die Verletzung zusätzlich zu beleidigen, haben wir dann viele Argumente, ob private Methoden Unit-getestet werden sollten oder nicht! (natürlich sollten sie, was '

Das Problem liegt dann natürlich in der Art des Testwerkzeugs.

Es gibt jetzt bessere Tools, mit denen diese Designänderungen für immer behoben werden können. Microsoft verfügt über Fälschungen (nee Moles), mit denen Sie konkrete, auch statische Objekte entfernen können, sodass Sie den Code nicht mehr ändern müssen, um das Tool anzupassen. In Ihrem Fall würden Sie, wenn Sie Fakes verwenden, den GetTypes-Aufruf durch Ihren eigenen ersetzen, der gültige und ungültige Testdaten zurückgibt - was ziemlich wichtig ist, da Ihre vorgeschlagene Änderung dies überhaupt nicht vorsieht.

Zur Antwort: Ihr Kollege hat Recht, aber möglicherweise aus den falschen Gründen. Ändern Sie nicht den Code, um zu testen, oder ändern Sie Ihr Testwerkzeug (oder Ihre gesamte Teststrategie, um mehr Unit-Tests im Integrationsstil zu erhalten, anstatt solche detaillierten Tests durchzuführen).

Martin Fowler hat eine Diskussion in diesem Bereich in seinem Artikel Mocks don't Stubs

Es ist eine gute und gebräuchliche Praxis, Unit-Tests und Debug-Protokolle zu verwenden . Unit-Tests stellen sicher, dass Ihre alte Funktionalität nicht kaputt geht, wenn Sie weitere Programmänderungen vornehmen. Mithilfe von Debug-Protokollen können Sie das Programm zur Laufzeit verfolgen.
Manchmal kommt es vor, dass wir darüber hinaus etwas nur zu Testzwecken benötigen. Es ist nicht ungewöhnlich, den Code dafür zu ändern. Es ist jedoch darauf zu achten, dass der Produktionscode dadurch nicht beeinflusst wird. In C ++ und C wird dies mithilfe von MACRO erreicht , einer Entität zur Kompilierungszeit. Dann wird der Testcode in der Produktionsumgebung überhaupt nicht sichtbar. Ich weiß nicht, ob es eine solche Bestimmung in C # gibt.
Auch wenn Sie Ihrem Programm Testcode hinzufügen, sollte dieser deutlich sichtbar seindass dieser Teil des Codes zu Testzwecken hinzugefügt wird. Oder der Entwickler, der versucht, den Code zu verstehen, wird einfach über diesen Teil des Codes schwitzen.

Es gibt einige gravierende Unterschiede zwischen Ihren Beispielen. Im Fall von DoSomethingOnAllTypes()gibt es eine Implikation, do somethingdie für Typen in der aktuellen Assembly gilt. Aber DoSomething(Assembly asm)zeigt explictly , dass Sie passieren kann jede Baugruppe zu.

Der Grund, warum ich darauf hinweise, ist, dass viele Abhängigkeitsinjektionen nur zu Testzwecken außerhalb der Grenzen des ursprünglichen Objekts liegen. Ich weiß, dass Sie gesagt haben " nicht alles öffentlich machen ", aber das ist einer der größten Fehler dieses Modells, dicht gefolgt von diesem: Öffnen der Methoden des Objekts für Zwecke, für die sie nicht bestimmt sind.

Ihre Frage gab nicht viel Kontext, in dem Ihr Kollege argumentierte, so gibt es Raum für Spekulationen

"schlechte Praxis" oder nicht, hängt davon ab, wie und wann die Änderungen vorgenommen werden.

Nach meiner Meinung ist Ihr Beispiel, eine Methode zu extrahieren, DoSomething(types)in Ordnung.

Aber ich habe Code gesehen, der so nicht in Ordnung ist :

public void DoSomethingOnAllTypes()
{
  var types = (!testmode) 
      ? Assembly.GetExecutingAssembly().GetTypes() 
      : getTypesFromTestgenerator();

  foreach (var currentType in types)
  {
     if (!testmode)
     {
        // do something with this type that made the unittest fail and should be skipped.
     }
     // do something with this type (e.g: read it's attributes, process, etc).
  }
}

Diese Änderungen haben das Verständnis des Codes erschwert, da Sie die Anzahl der möglichen Codepfade erhöht haben.

Was ich meine mit wie und wann :

Wenn Sie eine funktionsfähige Implementierung haben und im Interesse der "Implementierung von Testfunktionen" die Änderungen vorgenommen haben, müssen Sie Ihre Anwendung erneut testen, da Sie möglicherweise Ihre DoSomething()Methode beschädigt haben .

Das if (!testmode)ist schwieriger zu verstehen und zu testen als die extrahierte Methode.