So entfernen Sie eine Funktion oder ein Merkmal bei Verwendung von TDD

20

In Texten über TDD las ich oft über "Entfernen von Duplikaten" oder "Verbessern der Lesbarkeit" während des Refactoring-Schritts. Aber warum entferne ich eine nicht verwendete Funktion?

Nehmen wir zum Beispiel an, es gibt eine Klasse Cmit Methoden a()und b(). Jetzt denke ich, es wäre schön, eine Methode zu haben, in f()die gefahren wird C. Tatsächlich f()ersetzt alle Anrufe b()mit Ausnahme der Unit-Tests, die definiert / beschrieben wurden b(). Es wird nicht mehr benötigt - außer für die Tests.

Ist es sicher, nur b()alle Tests zu entfernen , die es verwendet haben? Gehört das zu "Lesbarkeit verbessern"?

TobiMcNamobi
quelle
3
Fügen Sie einfach einen weiteren Test hinzu, um zu überprüfen, ob die Funktion nicht vorhanden ist, und beheben Sie die fehlerhaften Testfälle;)
Filip Haglund
@FilipHaglund Je nach Sprache ist dies möglich. Aber als Codedokumentation würde das komisch aussehen.
TobiMcNamobi
1
Nur für alle, die es vielleicht nicht besser wissen: @ FilipHaglunds Kommentar ist offensichtlich ein Witz. Mach das nicht.
Jhyot

Antworten:

16

Ja natürlich. Der am einfachsten zu lesende Code ist der, der nicht vorhanden ist.

Das heißt, Refactoring bedeutet im Allgemeinen, den Code zu verbessern, ohne sein Verhalten zu ändern. Wenn Sie an etwas denken, das den Code verbessert, tun Sie es einfach. Es ist nicht nötig, es in ein Taubenloch zu stecken, bevor Sie es tun dürfen.

Sebastian Redl
quelle
@ jpmc26 Agil, Mann, agil! :-)
TobiMcNamobi
1
"Der am einfachsten zu lesende Code ist der, der nicht vorhanden ist." - Ich
hänge
27

Das Entfernen einer öffentlichen Methode ist kein "Refactoring" - das Refactoring ändert die Implementierung, während weiterhin vorhandene Tests bestanden werden.

Das Entfernen einer nicht benötigten Methode ist jedoch eine durchaus sinnvolle Designänderung.

TDD macht dies in gewissem Maße deutlich, da Sie bei der Überprüfung der Tests möglicherweise feststellen, dass es sich um eine nicht benötigte Methode handelt. Die Tests treiben Ihr Design voran, denn Sie können sagen: "Sehen Sie, dieser Test hat nichts mit meinem Ziel zu tun."

In Verbindung mit Code-Coverage-Tools kann es sich bei höheren Teststufen als besser erweisen. Wenn Sie Integrationstests mit Codeabdeckung ausführen und feststellen, dass keine Methoden aufgerufen werden, ist dies ein Hinweis darauf, dass eine Methode nicht verwendet wird. Eine statische Codeanalyse kann auch anzeigen, dass keine Methoden verwendet werden.

Es gibt zwei Ansätze zum Entfernen einer Methode. beide arbeiten unter verschiedenen Umständen:

  1. Löschen Sie die Methode. Folgen Sie den Kompilierungsfehlern, um abhängigen Code und Tests zu löschen. Wenn Sie zufrieden sind, dass die betroffenen Tests verfügbar sind, übernehmen Sie Ihre Änderungen. Wenn nicht, rollen Sie zurück.

  2. Löschen Sie die Tests, die Sie für veraltet halten. Führen Sie Ihre gesamte Testsuite mit Codeabdeckung aus. Löschen Sie Methoden, die von der Testsuite nicht ausgeführt wurden.

(Dies setzt voraus, dass Ihre Testsuite zunächst eine gute Abdeckung aufweist.)

schlank
quelle
10

Tatsächlich ersetzt f () alle Aufrufe von b () mit Ausnahme der Komponententests, die b () definiert / beschrieben haben.

IMHO wird der typische TDD-Zyklus so aussehen:

  • schreibe fehlgeschlagene Tests für f () (wahrscheinlich basierend auf den Tests für b ()): Tests werden rot

  • implementiere f () -> tests werden grün

  • refactor : -> entferne b () und alle Tests für b ()

Als letzten Schritt sollten Sie in Betracht ziehen, zuerst b () zu entfernen und zu sehen, was passiert (wenn Sie eine kompilierte Sprache verwenden, sollte der Compiler sich nur über die vorhandenen Tests beschweren, andernfalls schlagen die alten Komponententests für b fehl klar muss man sie auch entfernen).

Doc Brown
quelle
4

Ja ist es.

Der beste, fehlerfreieste und lesbarste Code ist der Code, den es nicht gibt. Bemühen Sie sich, so viel Nicht-Code wie möglich zu schreiben, während Sie Ihre Anforderungen erfüllen.

Kilian Foth
quelle
9
Sie haben das "How to" verpasst.
JeffO
2

Es ist wünschenswert, diese zu entfernen, b()sobald sie nicht mehr verwendet wird, aus dem gleichen Grund, aus dem es wünschenswert ist, nicht verwendete Funktionen überhaupt nicht hinzuzufügen. Egal, ob Sie es "Lesbarkeit" oder etwas anderes nennen, alles andere als gleich ist es eine Verbesserung des Codes, dass es nichts enthält, wofür es keine Verwendung hat. Um mindestens eine bestimmte Maßnahme zu haben, mit der es besser ist, sie nicht zu haben, garantiert das Entfernen, dass die zukünftigen Wartungskosten nach dieser Änderung Null sind!

Ich habe keine spezielle Technik gefunden, um es mit seinen Tests tatsächlich zu entfernen, da jeder Gedanke, es b()durch etwas Neues zu ersetzen, natürlich von einer Betrachtung des gesamten Codes begleitet sein muss, der gerade aufgerufen wird b(), und Tests eine Teilmenge des gesamten Codes sind ".

Die Argumentationslinie , die für mich im Allgemeinen funktioniert , ist an dem Punkt, wo ich merke , dass f()gemacht hat b()veraltet, daher b()zumindest als veraltet werden soll, und ich bin auf der Suche alle Anrufe zu finden , b()mit der Absicht , sie mit Anrufen zu ersetzen f(), ich Beachten Sie auch den Testcode . Insbesondere, wenn b()es nicht mehr benötigt wird, kann und sollte ich seine Komponententests entfernen.

Sie haben völlig recht, dass mich nichts dazu zwingt , das zu bemerken, b()was nicht mehr benötigt wird. Das ist eine Frage des Könnens (und, wie schon gesagt, der Code-Coverage-Bericht über Tests auf höherer Ebene). Wenn sich nur Unit-Tests und keine Funktionstests darauf beziehen b(), kann ich vorsichtig optimistisch sein, dass es nicht Teil einer veröffentlichten Schnittstelle ist, und daher ist das Entfernen keine bahnbrechende Änderung für Code, der nicht unter meiner direkten Kontrolle steht.

Im Rot / Grün / Refaktor-Zyklus wird das Entfernen von Tests nicht explizit erwähnt. Die Weitere Entfernen b()verletzt das offene / geschlossene Prinzip da eindeutig die Komponente ist für die Modifikation offen. Wenn Sie sich diesen Schritt also als etwas außerhalb des einfachen TDD vorstellen möchten, fahren Sie fort. Zum Beispiel könnten Sie ein Verfahren haben, um einen Test als "schlecht" zu deklarieren, das in diesem Fall angewendet werden kann, um den Test mit der Begründung zu entfernen, dass er auf etwas prüft, das nicht vorhanden sein sollte (die unnötige Funktion b()).

Ich denke, in der Praxis erlauben die meisten Leute wahrscheinlich, dass ein gewisses Maß an Neugestaltung zusammen mit einem Rot / Grün / Refaktor-Zyklus durchgeführt wird, oder sie betrachten das Entfernen redundanter Komponententests als einen gültigen Teil eines "Refaktors", obwohl dies streng genommen ist Es ist kein Refactoring. Ihr Team kann entscheiden, wie viel Drama und Papierkram erforderlich sind, um diese Entscheidung zu rechtfertigen.

Jedenfalls, wenn b()es wichtig wäre, gäbe es Funktionstests dafür, und diese würden nicht leichtfertig entfernt, aber Sie haben bereits gesagt, dass es nur Komponententests gibt. Wenn Sie nicht richtig zwischen Komponententests (geschrieben in das aktuelle interne Design des Codes, das Sie geändert haben) und Funktionstests (geschrieben in veröffentlichte Schnittstellen, die Sie möglicherweise nicht ändern möchten) unterscheiden, müssen Sie vorsichtiger sein über Unit-Tests entfernen.

Steve Jessop
quelle
2

Eine Sache, an die Sie sich immer erinnern sollten, ist, dass wir jetzt CODE REPOSITORIES mit VERSION CONTROL verwenden. Dieser gelöschte Code ist nicht wirklich verschwunden ... er ist noch irgendwo in einer vorherigen Iteration vorhanden. Also weg damit! Seien Sie liberal mit der Entf-Taste, denn Sie können jederzeit zurückgehen und die kostbare elegante Methode aufrufen, die Sie für nützlich hielten ... falls diese eines Tages jemals eintreten sollte. Es ist da.

Dies geht natürlich einher mit der Einschränkung der Nachteile und der Gefahr nicht abwärtskompatibler Releases ... externer Anwendungen, die auf Ihrer Schnittstellenimplementierung beruhten und die jetzt von Ihrem (plötzlich) veralteten Code verwaist werden.

Dwoz
quelle
Das Löschen von Code ist im Allgemeinen kein Problem. Ich liebe es zu löschen, um Zeilen, Funktionen oder ... nun, ganze Module zu löschen! Wenn möglich. Und das alles mache ich mit oder ohne TDD. Aber: Gibt es einen Punkt im TDD-Workflow, an dem (nicht duplizierter) Code gelöscht wird? Wenn nicht, wird es trotzdem gelöscht. Aber ist da? Das war meine frage
TobiMcNamobi