Wann wird das „Do One Thing“ -Paradigma schädlich?

21

Aus Gründen der Argumentation ist hier eine Beispielfunktion, die den Inhalt einer bestimmten Datei zeilenweise ausgibt.

Version 1:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  string line;
  while (std::getline(file, line)) {
    cout << line << endl;
  }
}

Ich weiß, es wird empfohlen, dass Funktionen eine Sache auf einer Abstraktionsebene ausführen. Obwohl der obige Code so ziemlich eine Sache macht und ziemlich atomar ist.

Einige Bücher (wie Robert C. Martins Clean Code) scheinen zu empfehlen, den obigen Code in separate Funktionen aufzuteilen.

Version 2:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  printLines(file);
}

void printLines(fstream & file) {
  string line;
  while (std::getline(file, line)) {
    printLine(line);
  }
}

void printLine(const string & line) {
  cout << line << endl;
}

Ich verstehe, was sie erreichen wollen (Datei öffnen / Zeilen lesen / Zeile drucken), aber ist es nicht ein bisschen übertrieben?

Die Originalversion ist einfach und macht in gewisser Weise bereits eine Sache - druckt eine Datei.

Die zweite Version wird zu einer großen Anzahl wirklich kleiner Funktionen führen, die möglicherweise weitaus weniger lesbar sind als die erste Version.

Wäre es in diesem Fall nicht besser, den Code an einem Ort zu haben?

Ab wann wird das "Do One Thing" -Paradigma schädlich?

Petr
quelle
13
Diese Art der Codierung erfolgt immer von Fall zu Fall. Es gibt niemals einen einzigen Ansatz.
iammilind
1
@Alex - Die akzeptierte Antwort hat buchstäblich nichts mit der Frage zu tun. Ich finde das wirklich seltsam.
ChaosPandion
2
Ich stelle fest, dass Ihre überarbeitete Version auf dem Kopf steht, was zu mangelnder Lesbarkeit führt. Lese auf die Datei würde man erwarten , um zu sehen printFile, printLinesund schließlich printLine.
Anthony Pegram
1
@ Kev, ich kann noch einmal nur widersprechen, besonders mit dieser Kategorisierung. Es ist keine Pedanterie, es ist der Punkt! Es ist das OP, das speziell besagt, dass die zweite Version möglicherweise nicht so lesbar ist. Es ist das OP, das Clean Code ausdrücklich als Inspiration für die zweite Version zitiert. Mein Kommentar ist im Wesentlichen, dass Clean Code ihn nicht dazu bringen würde, Code auf diese Weise zu schreiben. Die Reihenfolge ist eigentlich wichtig für die Lesbarkeit. Sie lesen die Datei wie einen Zeitungsartikel und werden immer detaillierter, bis Sie im Grunde genommen desinteressiert sind.
Anthony Pegram
1
Wie Sie nicht erwarten würden, ein Gedicht rückwärts zu lesen, würden Sie auch nicht erwarten, die niedrigste Detailstufe als erstes in einer bestimmten Klasse zu sehen. Um Ihren Punkt, dies dauert Code wenig Zeit , um schnell sortieren durch, aber ich würde dieser Code nur annehmen , ist nicht der einzige Code , den er zu schreiben wird. Wenn er Clean Code zitiert, kann er es zumindest befolgen. Wenn der Code nicht in Ordnung ist, ist er mit Sicherheit weniger lesbar als sonst.
Anthony Pegram

Antworten:

15

Das wirft natürlich nur die Frage auf: "Was ist eine Sache?" Ist das Lesen einer Zeile eine Sache und das Schreiben einer Zeile eine andere? Oder ist das Kopieren einer Zeile von einem Stream in den anderen eine Sache? Oder eine Datei kopieren?

Darauf gibt es keine eindeutige, objektive Antwort. Es liegt an dir. Du kannst entscheiden. Du musst dich entscheiden. Das Hauptziel des "Do one thing" -Paradigmas besteht wahrscheinlich darin, Code zu erstellen, der so einfach wie möglich zu verstehen ist, sodass Sie ihn als Richtlinie verwenden können. Leider ist dies auch objektiv nicht messbar. Sie müssen sich also auf Ihr Bauchgefühl und die "WTF?" zählen in Code-Überprüfung .

IMO eine Funktion, die nur aus einer einzigen Codezeile besteht, ist selten die Mühe wert. Sie printLine()haben keinen Vorteil gegenüber der direkten Verwendung von std::cout << line << '\n'1 . Wenn ich sehe printLine(), muss ich entweder davon ausgehen, dass es tut, was der Name sagt, oder es nachschlagen und überprüfen. Wenn ich sehe std::cout << line << '\n', weiß ich sofort, was es tut, weil dies die kanonische Art ist, den Inhalt eines Strings als Zeile an auszugeben std::cout.

Ein weiteres wichtiges Ziel des Paradigmas ist es jedoch, die Wiederverwendung von Code zuzulassen, und das ist eine wesentlich objektivere Maßnahme. Zum Beispiel in der zweiten Version printLines() könnte so leicht geschrieben werden , dass es sich um ein universell nützlichen Algorithmus ist , dass Kopien Linien von einem Strom zum anderen:

void copyLines(std::istream& is, std::ostream& os)
{
  std::string line;
  while( std::getline(is, line) );
    os << line << '\n';
  }
}

Ein solcher Algorithmus könnte auch in anderen Zusammenhängen wiederverwendet werden.

Sie können dann alles, was für diesen einen Anwendungsfall spezifisch ist, in eine Funktion einfügen, die diesen generischen Algorithmus aufruft:

void printFile(const std::string& filePath) {
  std::ifstream file(filePath.c_str());
  printLines(file, std::cout);
}

1 Beachten Sie, dass ich '\n'eher als verwendet std::endl. '\n'sollte die Standardeinstellung für die Ausgabe einer neuen Zeile sein , std::endlist der ungerade Fall .

sbi
quelle
2
+1 - Ich stimme dem größtenteils zu, aber ich denke, es steckt mehr dahinter als "Bauchgefühl". Das Problem ist, wenn die Leute "eins" beurteilen, indem sie Implementierungsdetails zählen. Für mich sollte die Funktion eine einzige klare Abstraktion implementieren (und ihren Namen beschreiben). Sie sollten eine Funktion niemals "do_x_and_y" nennen. Die Implementierung kann und sollte mehrere (einfachere) Dinge tun - und jedes dieser einfacheren Dinge kann in mehrere noch einfachere Dinge zerlegt werden und so weiter. Es ist nur eine funktionale Zerlegung mit einer zusätzlichen Regel - dass die Funktionen (und ihre Namen) jeweils ein eindeutiges Konzept / eine eindeutige Aufgabe / was auch immer beschreiben sollen.
Steve314
@ Steve314: Ich habe Implementierungsdetails nicht als Möglichkeiten aufgeführt. Kopieren Linien von einem Stream zu einem anderen deutlich ist eine Ein-Sache Abstraktion. Oder ist es? Und es ist einfach zu vermeiden, do_x_and_y()indem Sie do_everything()stattdessen die Funktion benennen . Ja, das ist ein dummes Beispiel, aber es zeigt, dass diese Regel nicht einmal die extremsten Beispiele für schlechtes Design verhindert. IMO dies ist ein Bauchgefühl Entscheidung so viel wie ein von Konventionen diktiert. Andernfalls, wenn es objektiv wäre, könnten Sie sich eine Metrik dafür ausdenken - was Sie nicht können.
sbi
1
Ich wollte nicht widersprechen - nur eine Ergänzung vorschlagen. Ich denke, was ich vergessen habe, ist, dass von der Frage aus die Zerlegung in printLineusw. gültig ist - jede davon ist eine einzelne Abstraktion - aber das bedeutet nicht, dass es notwendig ist. printFileist schon "eins". Obwohl Sie dies in drei separate Abstraktionen auf niedrigerer Ebene zerlegen können, müssen Sie nicht auf jeder möglichen Abstraktionsebene zerlegen . Jede Funktion muss "eine Sache" machen, aber nicht jede mögliche "eine Sache" muss eine Funktion sein. Das Verschieben von zu viel Komplexität in den Aufrufgraphen kann selbst ein Problem sein.
Steve314
7

Wenn eine Funktion nur "eins" tut, ist dies ein Mittel zu zwei wünschenswerten Zwecken und kein Gebot Gottes:

  1. Wenn Ihre Funktion nur "eine Sache" tut, können Sie Code-Duplikationen und API-Aufblähungen vermeiden, da Sie Funktionen komponieren können, um komplexere Aufgaben zu erledigen, anstatt eine kombinatorische Explosion übergeordneter, weniger komponierbarer Funktionen zu haben .

  2. Wenn Funktionen nur "eine Sache" tun , wird der Code möglicherweise besser lesbar. Dies hängt davon ab, ob Sie durch das Entkoppeln von Dingen mehr Klarheit und Übersicht gewinnen, als durch die Ausführlichkeit, Indirektion und den konzeptionellen Aufwand der Konstrukte, mit denen Sie Dinge entkoppeln können, verloren gehen.

Daher ist "eins" unvermeidlich subjektiv und hängt davon ab, welche Abstraktionsebene für Ihr Programm relevant ist. Wenn dies printLinesals ein einziger grundlegender Vorgang und als die einzige Möglichkeit zum Drucken von Zeilen angesehen wird, die Sie interessieren oder für die Sie sich interessieren, dann printLinestut dies für Ihre Zwecke nur eine Sache. Sofern Sie die zweite Version nicht besser lesbar finden (ich nicht), ist die erste Version in Ordnung.

Wenn Sie anfangen, mehr Kontrolle über niedrigere Abstraktionsebenen zu benötigen und subtile Duplikationen und kombinatorische Explosionen (dh eine printLinesfür Dateinamen und eine vollständig separate printLinesfür fstreamObjekte, eine printLinesfür Konsole und eine printLinesfür Dateien) zu erzielen, dann printLinestun Sie auf dieser Ebene mehr als eine Sache der Abstraktion, die Sie interessieren.

dsimcha
quelle
Ich würde ein drittes hinzufügen und das sind kleinere Funktionen, die einfacher getestet werden können. Da wahrscheinlich weniger Eingaben erforderlich sind, wenn die Funktion nur eine Aufgabe erfüllt, ist es einfacher, sie unabhängig zu testen.
PersonalNexus
@PersonalNexus: Ich stimme dem Testproblem einigermaßen zu, aber es ist meiner Meinung nach albern, Implementierungsdetails zu testen. Für mich sollte ein Unit-Test "eine Sache" wie in meiner Antwort definiert testen. Alles, was feinkörniger ist, macht Ihre Tests spröde (weil sich die Implementierungsdetails ändern müssen) und Ihren Code nervig ausführlich, indirekt usw. (weil Sie Indirektion hinzufügen, nur um das Testen zu unterstützen).
dsimcha
6

In dieser Größenordnung spielt es keine Rolle. Die Implementierung mit nur einer Funktion ist völlig offensichtlich und verständlich. Das Hinzufügen von etwas mehr Komplexität macht es jedoch sehr attraktiv, die Iteration von der Aktion zu trennen. Angenommen, Sie müssen Zeilen aus einer Reihe von Dateien drucken, die durch ein Muster wie "* .txt" angegeben sind. Dann würde ich die Iteration von der Aktion trennen:

printLines(FileSet files) {
   files.each({ 
       file -> file.eachLine({ 
           line -> printLine(line); 
       })
   })
}

Jetzt kann die Datei-Iteration separat getestet werden.

Ich habe Funktionen aufgeteilt, um das Testen zu vereinfachen oder die Lesbarkeit zu verbessern. Wenn die Aktion, die für jede Datenzeile ausgeführt wird, komplex genug wäre, um einen Kommentar zu rechtfertigen, würde ich sie sicherlich in eine separate Funktion aufteilen.

Kevin Cline
quelle
4
Ich denke, du hast es geschafft. Wenn wir einen Kommentar zur Erläuterung einer Zeile benötigen, ist es immer an der Zeit, eine Methode zu extrahieren.
Roger CS Wernersson
5

Extrahieren Sie Methoden, wenn Sie das Gefühl haben, dass Sie einen Kommentar benötigen, um die Dinge zu erklären.

Schreiben Sie Methoden, die entweder nur auf offensichtliche Weise das tun, was der Name sagt, oder erzählen Sie eine Geschichte, indem Sie clever benannte Methoden aufrufen.

Roger CS Wernersson
quelle
3

Selbst in Ihrem einfachen Fall fehlen Ihnen Details, mit denen Sie nach dem Prinzip der einheitlichen Verantwortung besser umgehen können. Was passiert zum Beispiel, wenn beim Öffnen der Datei ein Fehler auftritt? Das Hinzufügen einer Ausnahmebehandlung zur Absicherung gegen Dateizugriffsrandfälle würde Ihrer Funktion 7-10 Codezeilen hinzufügen.

Nachdem Sie die Datei geöffnet haben, sind Sie immer noch nicht sicher. Es könnte von Ihnen weggerissen werden (vor allem, wenn es sich um eine Datei in einem Netzwerk handelt), Ihnen könnte der Speicher ausgehen, und es kann wieder eine Reihe von Randfällen auftreten, gegen die Sie sich absichern möchten, und Ihre monolithische Funktion aufblähen.

Die einzeilige Druckzeile scheint harmlos genug zu sein. Wenn jedoch neue Funktionen zum Dateidrucker hinzugefügt werden (Analysieren und Formatieren von Text, Rendern auf verschiedenen Anzeigearten usw.), wird dieser erweitert und Sie werden sich später bedanken.

Das Ziel von SRP ist es, Ihnen zu ermöglichen, über eine einzelne Aufgabe gleichzeitig nachzudenken. Es ist, als würde man einen großen Textblock in mehrere Absätze aufteilen, damit der Leser den Punkt verstehen kann, den Sie vermitteln möchten. Das Schreiben von Code, der diesen Grundsätzen entspricht, dauert etwas länger. Dadurch wird es jedoch einfacher, diesen Code zu lesen. Überlegen Sie, wie glücklich Ihr zukünftiges Ich sein wird, wenn er einen Fehler im Code aufspüren muss und ihn ordentlich partitioniert findet.

Michael Brown
quelle
2
Ich habe diese Antwort positiv bewertet, weil mir die Logik gefällt, obwohl ich damit nicht einverstanden bin! Die Bereitstellung einer Struktur auf der Grundlage komplexer Überlegungen, was in Zukunft passieren könnte, ist kontraproduktiv. Faktorisieren Sie den Code, wenn Sie ihn benötigen. Abstrahieren Sie nicht, bis Sie müssen. Moderner Code wird von Leuten geplagt, die versuchen, Regeln sklavisch zu befolgen, anstatt nur Code zu schreiben, der funktioniert, und ihn widerstrebend anzupassen . Gute Programmierer sind faul .
Yttrill
Danke für den Kommentar. Hinweis: Ich befürworte keine vorzeitige Abstraktion, sondern teile nur logische Operationen, damit dies später einfacher ist.
Michael Brown
2

Ich persönlich bevorzuge den letzteren Ansatz, weil er Ihnen die Arbeit in der Zukunft erspart und die Denkweise erzwingt, wie man es auf allgemeine Weise macht. Trotzdem ist Version 1 in Ihrem Fall besser als Version 2 - nur weil die mit Version 2 gelösten Probleme zu trivial und fstream-spezifisch sind. Ich denke, es sollte folgendermaßen gemacht werden (einschließlich der von Nawaz vorgeschlagenen Fehlerbehebung):

Allgemeine Hilfsprogrammfunktionen:

void printLine(ostream& output, const string & line) { 
    output << line << endl; 
} 

void printLines(istream& input, ostream& output) { 
    string line; 
    while (getline(input, line)) {
        printLine(output, line); 
    } 
} 

Domainspezifische Funktion:

void printFile(const string & filePath, ostream& output = std::cout) { 
    fstream file(filePath, ios::in); 
    printLines(file, output); 
} 

Jetzt printLinesund printLinekann nicht nur mit fstream, sondern mit jedem Stream arbeiten.

gwiazdorrr
quelle
2
Ich stimme dir nicht zu. Diese printLine()Funktion hat keinen Wert. Siehe meine Antwort .
sbi
1
Nun, wenn wir printLine () beibehalten, können wir einen Dekorator hinzufügen, der Zeilennummern oder Syntaxfarben hinzufügt. Allerdings würde ich diese Methoden erst extrahieren, wenn ich einen Grund dafür gefunden hätte.
Roger CS Wernersson
2

Jedes Paradigma (nicht unbedingt das, das Sie genannt haben), das befolgt werden soll, erfordert etwas Disziplin, und reduziert somit - zumindest, weil Sie es lernen müssen - den anfänglichen Overhead. In diesem Sinne kann jedes Paradigma schädlich werden, wenn die Kosten dieses Overheads nicht durch den Vorteil überkompensiert werden, den das Paradigma für sich behalten soll.

Die wahre Antwort auf die Frage erfordert daher eine gute Fähigkeit, die Zukunft "vorauszusehen", wie:

  • Ich bin im Moment dazu verpflichtet AundB
  • Wie hoch ist die Wahrscheinlichkeit, in einer nahen Zukunft werde ich auch erforderlich , zu tun A-und B+(dh etwas , das aussieht wie A und B, aber nur ein bisschen anders)?
  • Wie groß ist die Wahrscheinlichkeit, dass A + in ferner Zukunft zu A*oder wird A*-?

Wenn diese Wahrscheinlichkeit relativ hoch ist, ist es eine gute Chance, wenn ich - während ich über A und B nachdenke - auch über deren mögliche Varianten nachdenke, um so die gemeinsamen Teile zu isolieren, damit ich sie wiederverwenden kann.

Wenn diese Wahrscheinlichkeit sehr gering ist (welche Variante auch immer Aim Wesentlichen nichts anderes als sich Aselbst ist), untersuchen Sie, wie A weiter zerlegt wird, was höchstwahrscheinlich zu Zeitverschwendung führt.

Lassen Sie mich als Beispiel diese wahre Geschichte erzählen:

In meinem früheren Leben als Lehrer entdeckte ich, dass - bei den meisten Schülerprojekten - praktisch alle ihre eigene Funktion zur Berechnung der Länge einer C-Saite bereitstellen .

Nach einigen Nachforschungen stellte ich fest, dass alle Schüler aufgrund des häufigen Problems auf die Idee kamen, eine Funktion dafür zu verwenden. Nachdem ihnen gesagt wurde, dass es eine Bibliotheksfunktion für das gibt ( strlen), antworteten viele von ihnen, dass das Problem so einfach und trivial sei, dass es für sie effektiver sei, ihre eigene Funktion (2 Codezeilen) zu schreiben, als das Handbuch der C-Bibliothek zu suchen (es war 1984, ich habe das WEB und google vergessen!) in strikter alphabetischer Reihenfolge, um zu sehen, ob es dafür eine Ready-Funktion gab.

Dies ist ein Beispiel, bei dem auch das Paradigma "Das Rad nicht neu erfinden" ohne einen wirksamen Radkatalog schädlich werden kann!

Emilio Garavaglia
quelle
2

Ihr Beispiel ist in Ordnung, um in einem Einwegwerkzeug verwendet zu werden, das gestern für eine bestimmte Aufgabe benötigt wird. Oder als Verwaltungstool, das direkt von einem Administrator gesteuert wird. Machen Sie es jetzt robust, für Ihre Kunden geeignet zu sein.

Fügen Sie eine ordnungsgemäße Fehler- / Ausnahmebehandlung mit aussagekräftigen Meldungen hinzu. Möglicherweise benötigen Sie eine Parameterüberprüfung, einschließlich der Entscheidungen, die getroffen werden müssen, z. B. wie mit nicht vorhandenen Dateien umgegangen werden soll. Fügen Sie Protokollierungsfunktionen hinzu, möglicherweise mit verschiedenen Ebenen wie Info und Debugging. Fügen Sie Kommentare hinzu, damit Ihre Teamkollegen wissen, was dort vor sich geht. Fügen Sie alle Teile hinzu, die in der Regel der Kürze halber weggelassen wurden und dem Leser als Übung dienen, wenn Sie Codebeispiele angeben. Vergessen Sie nicht Ihre Unit-Tests.

Ihre schön und recht linear kleine Funktion plötzlich endet in einem komplexen Chaos , dass bettelt in separate Funktionen aufgeteilt werden.

Sichern
quelle
2

IMO wird es schädlich, wenn es so weit geht, dass eine Funktion kaum etwas anderes tut, als Arbeit an eine andere Funktion zu delegieren, weil dies ein Zeichen dafür ist, dass es keine Abstraktion von irgendetwas mehr ist und die Denkweise, die zu solchen Funktionen führt, immer in Gefahr ist Schlimmeres tun ...

Aus dem ursprünglichen Beitrag

void printLine(const string & line) {
  cout << line << endl;
}

Wenn Sie pedantisch genug sind, werden Sie feststellen, dass printLine immer noch zwei Dinge tut: Schreiben Sie die Zeile in cout und fügen Sie ein "Endzeilen" -Zeichen hinzu. Einige Leute möchten vielleicht damit umgehen, indem sie neue Funktionen erstellen:

void printLine(const string & line) {
  reallyPrintLine(line);
  addEndLine();
}

void reallyPrintLine(const string & line) {
  cout << line;
}

void addEndLine() {
  cout << endl;
}

Oh nein, jetzt haben wir das Problem noch schlimmer gemacht! Jetzt ist es sogar klar, dass printLine ZWEI Dinge tut !!! 1! Es macht nicht viel Dummheit, die absurdesten "Umgehungsmöglichkeiten" zu schaffen, die man sich vorstellen kann, um das unvermeidliche Problem loszuwerden, dass das Drucken einer Zeile darin besteht, die Zeile selbst zu drucken und ein Zeilenendezeichen hinzuzufügen.

void printLine(const string & line) {
  for (int i=0; i<2; i++)
    reallyPrintLine(line, i);
}

void reallyPrintLine(const string & line, int action) {
  cout << (action==0?line:endl);
}
user281377
quelle
1

Kurze Antwort ... es kommt darauf an.

Denken Sie darüber nach: Was ist, wenn Sie in Zukunft nicht mehr nur auf Standardausgabe drucken möchten, sondern auf eine Datei.

Ich weiß, was YAGNI ist, aber ich sage nur, dass es Fälle geben kann, in denen einige Implementierungen bekanntermaßen erforderlich sind, die jedoch verschoben wurden. Vielleicht muss der Architekt oder was auch immer weiß, dass diese Funktion in der Lage sein muss, auch in eine Datei zu drucken, aber er möchte die Implementierung jetzt nicht durchführen. Er erstellt diese zusätzliche Funktion, sodass Sie in Zukunft nur noch die Ausgabe an einer Stelle ändern müssen. Macht Sinn?

Wenn Sie jedoch sicher sind, dass Sie nur eine Ausgabe in der Konsole benötigen, ist dies nicht wirklich sinnvoll. Das Überschreiben eines "Wrappers" cout <<erscheint nutzlos.

Luchian Grigore
quelle
1
Ist die printLine-Funktion streng genommen nicht eine andere Abstraktionsebene als das Iterieren über Zeilen?
@Petr Ich denke schon, weshalb sie vorschlagen, dass Sie die Funktionalität trennen. Ich denke, das Konzept ist richtig, aber Sie müssen es von Fall zu Fall anwenden.
1

Der ganze Grund, warum es Bücher gibt, die Kapitel über die Tugenden von "Mach eins" widmen, ist, dass es immer noch Entwickler gibt, die Funktionen schreiben, die 4 Seiten lang sind und die Bedingungen 6 Stufen erfüllen. Wenn Ihr Code einfach und klar ist, haben Sie es richtig gemacht.

Kevin
quelle
0

Wie andere Poster kommentiert haben, ist es eine Frage des Maßstabs, etwas zu tun.

Ich würde auch vorschlagen, dass die One Thing-Idee darin besteht, Menschen davon abzuhalten, nebenbei zu programmieren. Ein Beispiel hierfür ist die sequentielle Kopplung, bei der Methoden in einer bestimmten Reihenfolge aufgerufen werden müssen, um das richtige Ergebnis zu erzielen.

NWS
quelle