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

77

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?

liortal
quelle
5
Ihre geänderte Methode sollte a Typeals Parameter und nicht als Assembly.
Robert Harvey
Sie haben Recht - Typ oder Assembly, der Punkt war, dass der Code die Angabe dieses Parameters zulassen sollte, obwohl es den Anschein haben mag, dass diese Funktionalität nur zum Testen dient ...
Datum
5
Ihr Auto hat einen ODBI-Port zum "Testen" - wenn alles perfekt wäre, würde es nicht benötigt. Meiner Meinung nach ist Ihr Auto zuverlässiger als seine Software.
Mattnz
24
Welche Gewissheit hat er, dass der Code "funktioniert"?
Craige
3
Natürlich - tu es. Getesteter Code ist über die Zeit stabil. Aber Sie werden es viel einfacher haben, neu eingeführte Fehler zu erklären, wenn sie mit einer Funktionsverbesserung statt einer Korrektur für die Testbarkeit einhergingen
Petter Nordlander,

Antworten:

135

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.
Robert Harvey
quelle
3
Ich habe diese Dinge in meiner Antwort nicht erwähnt, aber ich stimme vollkommen zu. Wenn Sie Ihren Code so überarbeiten, dass er besser getestet werden kann, kann dies mehrere erfreuliche Nebenwirkungen haben.
Jason Swett
2
+1: Ich stimme im Allgemeinen zu. Es gibt zweifellos eindeutige Fälle, in denen das Testen des Codes diesen anderen vorteilhaften Zielen schadet, und in diesen Fällen hat die Testbarkeit die niedrigste Priorität. Wenn Ihr Code zum Testen nicht flexibel / erweiterbar ist, ist er im Allgemeinen nicht flexibel / erweiterbar genug, um ihn ordnungsgemäß zu verwenden oder zu altern.
Telastyn
6
Testbarer Code ist besserer Code. Es ist immer ein Risiko, Code zu modifizieren, der keine Tests enthält. Die einfachste und kostengünstigste Möglichkeit, dieses Risiko kurzfristig zu minimieren, besteht darin, ihn in Ruhe zu lassen, was in Ordnung ist ... bis Sie ihn tatsächlich benötigen um den Code zu ändern. Sie müssen lediglich die Vorteile von Unit-Tests an Ihren Kollegen verkaufen. Wenn jeder mit Unit-Tests an Bord ist, kann es kein Argument dafür geben, dass der Code testbar sein muss. Wenn nicht jeder mit Unit-Tests an Bord ist, hat das keinen Sinn.
guysherman
3
Genau. Ich erinnere mich, dass ich Unit-Tests für ungefähr 10.000 Quelltextzeilen meines eigenen Codes geschrieben habe. Ich wusste, dass der Code perfekt funktioniert, aber die Tests haben mich gezwungen, einige Situationen zu überdenken. Ich musste mich fragen "Was genau macht diese Methode?". Ich habe einige Fehler im Arbeitscode gefunden, indem ich ihn aus einem neuen Blickwinkel betrachtet habe.
Sulthan
2
Ich klicke weiter auf den Auf-Knopf, kann Ihnen aber nur einen Punkt geben. Mein jetziger Arbeitgeber ist zu kurzsichtig, um Unit-Tests durchführen zu können, und ich profitiere immer noch davon, wenn ich meinen Code bewusst so schreibe, als würde ich testgetrieben arbeiten.
AmericanUmlaut
59

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.

Mark Seemann
quelle
3
+1 Sie fühlen sich sicher, eine öffentliche Methode verfügbar zu machen, wenn sie ihre Invarianten ordnungsgemäß schützt.
Shambulator
1
Ich liebe deine Antwort! :) Wie oft höre ich: "Encapsulation ist der Prozess, um einige Methoden und Eigenschaften privat zu machen". Es ist so, als würde man sagen, wenn man objektorientiert programmiert, weil man mit Objekten programmiert. :( Ich bin nicht überrascht, dass eine so klare Antwort vom Meister der Abhängigkeitsinjektion kommt. Ich werde Ihre Antwort sicherlich ein paar Mal lesen, damit ich über meinen armen alten Legacy-Code lächle.
Samuel
Ich habe Ihrer Antwort einige Verbesserungen hinzugefügt. Abgesehen davon habe ich Ihren Beitrag Encapsulation of Properties gelesen. Der einzige zwingende Grund, den ich jemals für die Verwendung von Automatic Properties gehört habe, ist, dass das Ändern einer öffentlichen Variablen in eine öffentliche Eigenschaft die Binärkompatibilität beeinträchtigt. Wenn Sie es von Anfang an als automatische Eigenschaft verfügbar machen, können Sie die Validierung oder andere interne Funktionen später hinzufügen, ohne den Clientcode zu beschädigen.
Robert Harvey
21

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!

Jason Swett
quelle
DI ist nicht die Hauptfrage (ich habe nur versucht, ein Beispiel zu nennen), der Punkt war, ob es in Ordnung wäre, eine andere Methode bereitzustellen, die ursprünglich nicht für das Testen gedacht war.
22.
4
Ich weiß. Ich dachte, ich habe Ihren Hauptpunkt in meinem zweiten Absatz mit einem "Ja" angesprochen.
Jason Swett
13

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.

Carson63000
quelle
Ein wichtiges Anliegen. Normalerweise mache ich IDE-unterstützte Refactorings frei, da die Chance, etwas zu zerbrechen, dort sehr gering ist. Wenn das Refactoring mehr involviert ist, würde ich es nur tun, wenn ich den Code trotzdem ändern muss. Um Dinge zu ändern, müssen Sie Tests durchführen, um Risiken zu reduzieren, damit Sie sogar einen geschäftlichen Nutzen erzielen.
Hans-Peter Störr
Der Punkt ist: Wollen wir den alten Code behalten, weil wir dem Objekt die Verantwortung für die Abfrage der aktuellen Assembly übertragen wollen oder weil wir keine öffentlichen Eigenschaften hinzufügen oder die Methodensignatur ändern wollen? Ich denke, der Code verstößt gegen die SRP und aus diesem Grund sollte er überarbeitet werden, egal wie ängstlich die Kollegen sind. Sicher, wenn dies eine öffentliche API ist, die von vielen Benutzern verwendet wird, müssen Sie über eine Strategie nachdenken, wie z. B. die Implementierung einer Fassade oder irgendetwas, das Ihnen hilft, alten Code eine Schnittstelle zuzuweisen, die zu viele Änderungen verhindert.
Samuel
7

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.)

Aidan Cully
quelle
2

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.

ozz
quelle
2

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.

Sam Gamoran
quelle
2

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

gbjbaanb
quelle
1

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.

Manoj R
quelle
1

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.

Ross Patterson
quelle
0

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.

k3b
quelle