Mein Chef bittet mich, keine kleinen Funktionen mehr zu schreiben und alles in derselben Schleife zu machen

Ich habe ein Buch namens Clean Code von Robert C. Martin gelesen . In diesem Buch habe ich viele Methoden zum Aufräumen von Code gesehen, wie das Schreiben kleiner Funktionen, das sorgfältige Auswählen von Namen usw. Es scheint das mit Abstand interessanteste Buch über sauberen Code zu sein, das ich gelesen habe. Meinem Chef hat es heute jedoch nicht gefallen, wie ich nach dem Lesen dieses Buches Code geschrieben habe.

Seine Argumente waren

• Das Schreiben kleiner Funktionen ist ein Schmerz, da Sie gezwungen sind, in jede kleine Funktion zu wechseln, um zu sehen, was der Code tut.
• Setzen Sie alles in eine große Hauptschleife, auch wenn die Hauptschleife mehr als 300 Zeilen umfasst, ist das Lesen schneller.
• Schreiben Sie kleine Funktionen nur, wenn Sie Code duplizieren müssen.
• Schreiben Sie keine Funktion mit dem Namen des Kommentars, sondern setzen Sie Ihre komplexe Codezeile (3-4 Zeilen) mit einem Kommentar darüber. Ebenso können Sie den fehlerhaften Code direkt ändern

Dies ist gegen alles, was ich gelesen habe. Wie schreibt man normalerweise Code? Eine große Hauptschleife, keine kleinen Funktionen?

Die Sprache, die ich benutze, ist hauptsächlich Javascript. Ich habe jetzt wirklich Schwierigkeiten beim Lesen, da ich alle meine kleinen, klar benannten Funktionen gelöscht und alles in eine große Schleife gestellt habe. Mein Chef mag es jedoch so.

Ein Beispiel war:

// The way I would write it
if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

// The way he would write it
// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

In dem Buch, das ich zum Beispiel gelesen habe, werden Kommentare als Fehler beim Schreiben von sauberem Code angesehen, da sie veraltet sind, wenn Sie kleine Funktionen schreiben und häufig zu nicht aktualisierten Kommentaren führen (Sie ändern Ihren Code und nicht den Kommentar). Ich lösche jedoch den Kommentar und schreibe eine Funktion mit dem Namen des Kommentars.

Nun, ich hätte gerne einen Rat, auf welche Art und Weise / in welcher Praxis ist es besser, sauberen Code zu schreiben?

Nehmen Sie zuerst die Codebeispiele. Sie bevorzugen:

if (isApplicationInProduction(headers)) {
  phoneNumber = headers.resourceId;
} else {
  phoneNumber = DEV_PHONE_NUMBER;
}

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Und Ihr Chef würde es so schreiben:

// Take the right resourceId if application is in production
phoneNumber = headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;

Meiner Meinung nach haben beide Probleme. Als ich Ihren Code las, dachte ich sofort: "Sie können das ifdurch einen ternären Ausdruck ersetzen ." Dann las ich den Code Ihres Chefs und dachte: "Warum hat er Ihre Funktion durch einen Kommentar ersetzt?".

Ich würde vorschlagen, dass der optimale Code zwischen den beiden liegt:

phoneNumber = isApplicationInProduction(headers) ? headers.resourceId : DEV_PHONE_NUMBER;

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Das gibt Ihnen das Beste aus beiden Welten: Ein vereinfachter Testausdruck und der Kommentar wird durch testbaren Code ersetzt.

In Bezug auf die Ansichten Ihres Chefs zum Code-Design:

Das Schreiben kleiner Funktionen ist ein Schmerz, da Sie gezwungen sind, in die einzelnen kleinen Funktionen zu wechseln, um zu sehen, was der Code tut.

Wenn die Funktion gut benannt ist, ist dies nicht der Fall. isApplicationInProductionist selbstverständlich und es sollte nicht notwendig sein, den Code zu untersuchen, um zu sehen, was er tut. Tatsächlich ist das Gegenteil der Fall: Wenn Sie den Code untersuchen, werden Sie weniger über die Absicht informiert als über den Funktionsnamen (weshalb Ihr Chef auf Kommentare zurückgreifen muss).

Setzen Sie alles in eine große Hauptschleife, auch wenn die Hauptschleife mehr als 300 Zeilen umfasst, ist das Lesen schneller

Das Durchsuchen mag schneller sein, aber um den Code wirklich zu "lesen", müssen Sie ihn effektiv in Ihrem Kopf ausführen können. Das ist mit kleinen Funktionen einfach und mit Methoden, die Hunderte von Zeilen lang sind, wirklich sehr schwierig.

Schreiben Sie nur kleine Funktionen, wenn Sie Code duplizieren müssen

Ich stimme dir nicht zu. Wie Ihr Codebeispiel zeigt, verbessern kleine, gut benannte Funktionen die Lesbarkeit von Code und sollten immer dann verwendet werden, wenn Sie beispielsweise nicht am "Wie", sondern nur am "Was" einer Funktion interessiert sind.

Schreiben Sie keine Funktion mit dem Namen des Kommentars, sondern setzen Sie Ihre komplexe Codezeile (3-4 Zeilen) mit einem Kommentar darüber. Auf diese Weise können Sie den fehlerhaften Code direkt ändern

Ich kann die Gründe für diese wirklich nicht verstehen, vorausgesetzt, es ist wirklich ernst. Es ist die Art von Dingen, die ich von The Expert Beginner Twitter Account als Parodie erwarten würde . Kommentare haben einen fundamentalen Fehler: Sie werden nicht kompiliert / interpretiert und können daher nicht auf Einheit getestet werden. Der Code wird geändert und der Kommentar wird in Ruhe gelassen und Sie wissen am Ende nicht, welcher richtig ist.

Das Schreiben von selbstdokumentierendem Code ist schwierig, und manchmal sind zusätzliche Dokumente (auch in Form von Kommentaren) erforderlich. Aber die Ansicht von "Onkel Bob", dass Kommentare ein Codierungsfehler sind, trifft allzu oft zu.

Lassen Sie Ihren Chef das Clean Code-Buch lesen und versuchen Sie nicht, Ihren Code weniger lesbar zu machen, um ihn zufriedenzustellen. Letztendlich muss man sich aber entweder anstellen oder einen neuen Chef finden, der besser codieren kann, wenn man ihn nicht zum Wechseln überreden kann.

Es gibt noch andere Probleme

Keiner der Codes ist gut, da beide den Code im Grunde mit einem Debug-Testfall aufblähen . Was ist, wenn Sie aus irgendeinem Grund mehr Dinge testen möchten?

phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS;

oder

phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY;

Möchten Sie noch mehr Branchen hinzufügen?

Das Hauptproblem besteht darin, dass Sie im Grunde einen Teil Ihres Codes duplizieren und somit den tatsächlichen Code nicht testen. Sie schreiben Debug-Code, um den Debug-Code zu testen, jedoch nicht den Produktionscode. Es ist so, als würde man teilweise eine parallele Codebasis erstellen.

Sie streiten sich mit Ihrem Chef darüber, wie schlechter Code geschickter geschrieben werden kann. Stattdessen sollten Sie das inhärente Problem des Codes selbst beheben.

Abhängigkeitsspritze

So sollte Ihr Code aussehen:

phoneNumber = headers.resourceId;

Hier gibt es keine Verzweigung, weil die Logik hier keine Verzweigung hat. Das Programm sollte die Telefonnummer aus der Kopfzeile ziehen. Zeitraum.

Wenn Sie DEV_PHONE_NUMBER_FROM_OTHER_COUNTRYals Ergebnis haben möchten , sollten Sie es in setzen headers.resourceId. Eine Möglichkeit besteht darin, einfach ein anderes headersObjekt für Testfälle einzufügen (sorry, wenn dies nicht der richtige Code ist, sind meine JavaScript-Kenntnisse etwas verrostet):

function foo(headers){
    phoneNumber = headers.resourceId;
}

// Creating the test case
foo({resourceId: DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY});

Angenommen, dies headersist Teil einer Antwort, die Sie von einem Server erhalten: Idealerweise verfügen Sie über einen ganzen Testserver, der headerszu Testzwecken verschiedene Arten liefert . Auf diese Weise testen Sie den tatsächlichen Produktionscode wie er ist und keinen halb duplizierten Code, der wie der Produktionscode funktioniert oder nicht.

Darauf gibt es keine "richtige" oder "falsche" Antwort. Ich werde meine Meinung jedoch auf der Grundlage von 36 Jahren Berufserfahrung im Entwerfen und Entwickeln von Softwaresystemen abgeben ...

1. Es gibt keinen "selbstdokumentierenden Code". Warum? Weil dieser Anspruch völlig subjektiv ist.
2. Kommentare sind niemals Fehlschläge. Was ist ein Fehlercode, der gar nicht verstanden werden kann , ohne Kommentare.
3. 300 ununterbrochene Codezeilen in einem Codeblock sind ein Wartungsalptraum und sehr fehleranfällig. Ein solcher Block weist stark auf ein schlechtes Design und eine schlechte Planung hin.

Sprechen Sie direkt mit dem Beispiel, das Sie angegeben haben ... Sich isApplicationInProduction()in seine eigene Routine zu versetzen, ist die kluge Sache. Heutzutage ist dieser Test nur eine Überprüfung der "Header" und kann in einem ternary ( ?:) - Operator durchgeführt werden. Morgen ist der Test möglicherweise weitaus komplexer. Darüber hinaus hat "headers.resourceId" keine eindeutige Beziehung zum "Produktionsstatus" der Anwendung. Ich würde behaupten , dass ein Test für diesen Status muss von den zugrunde liegenden Daten entkoppelt werden; Eine Subroutine erledigt dies, eine Ternäre nicht. Außerdem würde ein hilfreicher Kommentar erklären, warum resourceId ein Test für "in Produktion" ist.

Achten Sie darauf, nicht mit "kleinen klar benannten Funktionen" über Bord zu gehen. Eine Routine sollte eine Idee mehr einschließen als "nur Code". Ich unterstütze den Vorschlag von amon phoneNumber = getPhoneNumber(headers)und füge hinzu, mit dem getPhoneNumber()der "Produktionsstatus" -Test durchgeführt werden sollisApplicationInProduction()

"Entitäten dürfen nicht über die Notwendigkeit hinaus multipliziert werden."

- Ockhams Rasiermesser

Code muss so einfach wie möglich sein. Bugs verstecken sich gerne zwischen Komplexität, weil sie dort schwer zu erkennen sind. Was macht Code einfach?

Kleine Einheiten (Dateien, Funktionen, Klassen) sind eine gute Idee . Kleine Einheiten sind einfach zu verstehen, weil Sie weniger Dinge auf einmal verstehen müssen. Normale Menschen können nur 7 Konzepte gleichzeitig unter einen Hut bringen. Die Größe wird jedoch nicht nur in Codezeilen gemessen . Ich kann so wenig Code wie möglich schreiben, indem ich den Code „spiele“ (indem ich kurze Variablennamen wähle, „clevere“ Abkürzungen nehme und so viel Code wie möglich in eine einzelne Zeile zerschmettere), aber das Endergebnis ist nicht einfach. Der Versuch, solchen Code zu verstehen, ähnelt eher dem Reverse Engineering als dem Lesen.

Eine Möglichkeit, eine Funktion zu verkürzen, besteht darin, verschiedene Hilfsfunktionen zu extrahieren. Das kann eine gute Idee sein, wenn es ein in sich geschlossenes Stück Komplexität extrahiert . Für sich genommen ist diese Komplexität viel einfacher zu verwalten (und zu testen!) Als wenn sie in ein nicht zusammenhängendes Problem eingebettet ist.

Aber jeder Funktionsaufruf hat einen kognitiven Overhead : Ich muss nicht nur den Code in meinem aktuellen Code verstehen, sondern auch verstehen, wie er von außen mit Code interagiert . Ich denke, es ist fair zu sagen, dass die von Ihnen extrahierte Funktion mehr Komplexität in die Funktion einbringt als sie extrahiert . Das ist es, was Ihr Chef unter " kleinen Funktionen" versteht, da Sie gezwungen sind, in jede kleine Funktion zu wechseln, um zu sehen, was der Code tut. "

Manchmal sind langweilige Funktionen unglaublich einfach zu verstehen, selbst wenn sie Hunderte von Zeilen lang sind. Dies tritt in der Regel im Initialisierungs- und Konfigurationscode auf, z. B. beim Erstellen einer grafischen Benutzeroberfläche von Hand ohne Drag-and-Drop-Editor. Es gibt kein eigenständiges Stück Komplexität, das Sie vernünftigerweise extrahieren könnten. Aber wenn die Formatierung lesbar ist und es einige Kommentare gibt, ist es nicht schwierig zu verfolgen, was passiert.

Es gibt viele andere Komplexitätsmetriken: Die Anzahl der Variablen in einem Bereich sollte so gering wie möglich sein. Das heißt nicht, dass wir Variablen vermeiden sollten . Das bedeutet, dass wir jede Variable auf den kleinstmöglichen Bereich beschränken sollten, in dem sie benötigt wird. Variablen werden auch einfacher, wenn wir den Wert, den sie enthalten, nie ändern.

Eine sehr wichtige Metrik ist die zyklomatische Komplexität (McCabe-Komplexität). Es misst die Anzahl unabhängiger Pfade durch ein Stück Code. Diese Zahl wächst exponentiell mit jeder Bedingung. Jede Bedingung oder Schleife verdoppelt die Anzahl der Pfade. Es gibt Hinweise darauf, dass eine Punktzahl von mehr als 10 zu komplex ist. Dies bedeutet, dass eine sehr lange Funktion, die möglicherweise eine Bewertung von 5 hat, möglicherweise besser ist als eine sehr kurze und dichte Funktion mit einer Bewertung von 25. Wir können die Komplexität reduzieren, indem wir den Steuerungsfluss in separate Funktionen extrahieren.

Ihre Bedingung ist ein Beispiel für eine Komplexität, die vollständig extrahiert werden könnte:

function bigFatFunction(...) {
  ...
  phoneNumber = getPhoneNumber(headers);
  ...
}

...

function getPhoneNumber(headers) {
  return headers.resourceId ? headers.resourceId : DEV_PHONE_NUMBER;
}

Dies ist immer noch sehr nützlich. Ich bin nicht sicher, ob dies die Komplexität wesentlich verringert, da diese Bedingung nicht sehr bedingt ist . In der Produktion wird es immer den gleichen Weg nehmen.

Komplexität kann niemals verschwinden. Es kann nur herumgemischt werden. Sind viele kleine Dinge einfacher als wenige große? Das hängt sehr stark von den Umständen ab. Normalerweise gibt es eine Kombination, die sich genau richtig anfühlt. Um diesen Kompromiss zwischen verschiedenen Komplexitätsfaktoren zu finden, sind Fingerspitzengefühl, Erfahrung und ein bisschen Glück erforderlich.

Zu wissen, wie man sehr kleine und sehr einfache Funktionen schreibt, ist eine nützliche Fähigkeit, da Sie keine Wahl treffen können, ohne die Alternativen zu kennen. Das blinde Befolgen von Regeln oder bewährten Methoden, ohne darüber nachzudenken, wie sie auf die aktuelle Situation angewendet werden, führt bestenfalls zu durchschnittlichen Ergebnissen, im schlimmsten Fall zu Frachtkult-Programmen .

Dort bin ich nicht mit deinem Chef einverstanden. Seine Argumente sind nicht ungültig, aber auch das Clean Code-Buch ist nicht falsch. Es ist wahrscheinlich besser, die Richtlinien Ihres Chefs zu befolgen, aber die Tatsache, dass Sie über diese Probleme nachdenken und versuchen, einen besseren Weg zu finden, ist sehr vielversprechend. Wenn Sie Erfahrung sammeln, fällt es Ihnen leichter, ein gutes Factoring für Ihren Code zu finden.

(Hinweis: Diese Antwort basiert zum Teil auf Gedanken aus dem Blogbeitrag von Jimmy Hoffa über vernünftigen Code auf dem Whiteboard , der eine allgemeine Übersicht darüber bietet, was Code einfach macht.)

Robert Martins Programmierstil ist polarisierend. Sie werden viele Programmierer finden, auch erfahrene, die eine Menge Ausreden finden, warum es "besser" ist, "so viel" aufzuteilen, und warum es "besser" ist, die Funktionen ein wenig größer zu halten. Die meisten dieser "Argumente" sind jedoch oft Ausdruck der Unwilligkeit, alte Gewohnheiten zu ändern und etwas Neues zu lernen.

Hör ihnen nicht zu!

Wann immer Sie einen Kommentar speichern können, indem Sie einen Teil des Codes in eine separate Funktion mit einem aussagekräftigen Namen umgestalten, tun Sie dies - es wird höchstwahrscheinlich Ihren Code verbessern. Sie müssen nicht so weit gehen, wie es Bob Martin in seinem sauberen Codebuch tut, aber die große Mehrheit des Codes, den ich in der Vergangenheit gesehen habe, die Wartungsprobleme verursacht hat, enthielt zu große Funktionen, nicht zu kleine. Versuchen Sie also, kleinere Funktionen mit selbstbeschreibenden Namen zu schreiben.

Automatische Refactoring-Tools machen das Extrahieren von Methoden einfach, einfach und sicher. Und bitte nehmen Sie Leute nicht ernst, die das Schreiben von Funktionen mit mehr als 300 Zeilen empfehlen - solche Leute sind definitiv nicht qualifiziert, Ihnen zu sagen, wie Sie codieren sollen.

In Ihrem Fall: Sie möchten eine Telefonnummer. Entweder ist es offensichtlich, wie Sie eine Telefonnummer bekommen würden, dann schreiben Sie den offensichtlichen Code. Oder es ist nicht klar, wie Sie eine Telefonnummer bekommen würden, dann schreiben Sie eine Methode dafür.

In Ihrem Fall ist es nicht offensichtlich, wie Sie die Telefonnummer erhalten, also schreiben Sie eine Methode dafür. Die Implementierung ist nicht offensichtlich, aber aus diesem Grund stellen Sie sie in eine separate Methode, sodass Sie sich nur einmal damit befassen müssen. Ein Kommentar wäre hilfreich, da die Implementierung nicht offensichtlich ist.

Die Methode "isApplicationInProduction" ist ziemlich sinnlos. Wenn Sie es von Ihrer getPhonenumber-Methode aufrufen, wird die Implementierung nicht offensichtlicher und es wird nur schwieriger, herauszufinden, was los ist.

Schreiben Sie keine kleinen Funktionen. Schreiben Sie Funktionen, die einen genau definierten Zweck haben und diesen genau definierten Zweck erfüllen.

PS. Die Umsetzung gefällt mir überhaupt nicht. Es wird davon ausgegangen, dass das Fehlen einer Telefonnummer bedeutet, dass es sich um eine Dev-Version handelt. Wenn die Telefonnummer in der Produktion nicht vorhanden ist, wird sie nicht nur nicht verarbeitet, sondern durch eine zufällige Telefonnummer ersetzt. Stellen Sie sich vor, Sie haben 10.000 Kunden, 17 haben keine Telefonnummer und Sie haben Probleme in der Produktion. Ob Sie in der Produktion oder in der Entwicklung sind, sollte direkt überprüft werden, nicht von etwas anderem abgeleitet.

Selbst wenn ich die Tatsache ignoriere, dass keine der beiden Implementierungen so gut ist, stelle ich fest, dass dies im Wesentlichen eine Geschmacksfrage ist, zumindest auf der Ebene der Abstraktion von Einweg-Trivialfunktionen.

Die Anzahl der Zeilen ist in den meisten Fällen keine nützliche Metrik.

300 (oder sogar 3000) Zeilen völlig trivialer, rein sequentieller Code (Setup oder ähnliches) sind selten ein Problem (aber möglicherweise besser automatisch generiert oder als Datentabelle oder ähnliches), 100 Zeilen verschachtelter Schleifen mit vielen komplizierten Ausgangsbedingungen und Mathematik, wie sie in der Gaußschen Eliminierung oder Matrixinversion zu finden sind, sind möglicherweise viel zu viel, um sie einfach nachzuvollziehen.

Für mich würde ich keine Funktion zur einmaligen Verwendung schreiben, es sei denn, die für die Deklaration des Objekts erforderliche Codemenge wäre viel geringer als die für die Implementierung erforderliche Codemenge (es sei denn, ich hätte Grund zu der Annahme, dass ich die Fehlerinjektion einfach durchführen kann). Eine einzelne Bedingung passt selten zu dieser Rechnung.

Jetzt komme ich aus einer kleinen Kern-Embedded-Welt, in der wir auch Dinge wie Stack-Tiefe und Call / Return-Overheads berücksichtigen müssen (was wiederum gegen die Art winziger Funktionen spricht, die hier befürwortet zu werden scheinen), und dies könnte mein Design verzerren Entscheidungen, aber wenn ich diese ursprüngliche Funktion in einer Codeüberprüfung sehen würde, würde dies eine alte Usenet-Flamme als Antwort erhalten.

Geschmack ist Design ist schwer zu lehren und kommt nur mit Erfahrung, ich bin nicht sicher, ob es sich auf Regeln über Funktionslängen reduzieren lässt, und selbst die zyklomatische Komplexität hat ihre Grenzen als Metrik (manchmal sind die Dinge einfach kompliziert, wie man sie auch angeht).
Dies soll nicht heißen, dass sauberer Code nicht über einige gute Dinge spricht, sondern dies tut. Diese Dinge sollten bedacht werden, aber lokale Gewohnheiten und die Bedeutung der vorhandenen Codebasis sollten ebenfalls berücksichtigt werden.

Dieses spezielle Beispiel scheint mir triviale Details aufzuzeigen. Ich würde mich mehr um Dinge auf höherer Ebene kümmern, da dies weitaus mehr für die Fähigkeit von Bedeutung ist, ein System leicht zu verstehen und zu debuggen.

Stecke nicht alles in eine große Schleife, aber mache das auch nicht zu oft:

function isApplicationInProduction(headers) {
   return _.has(headers, 'resourceId');
}

Das Problem mit der großen Schleife ist, dass es sehr schwer ist, ihre Gesamtstruktur zu sehen, wenn sie sich über viele Bildschirme erstreckt. Versuchen Sie also, große Stücke herauszunehmen, idealerweise Stücke, die eine einzige Verantwortung haben und wiederverwendbar sind.

Das Problem mit der oben genannten winzigen Funktion ist, dass Atomizität und Modularität im Allgemeinen gut sind, aber zu weit gehen können. Wenn Sie die oben genannte Funktion nicht wiederverwenden, wird die Lesbarkeit und Wartbarkeit des Codes beeinträchtigt. Um einen Drilldown in das Detail durchzuführen, müssen Sie zur Funktion springen, anstatt das Detail inline lesen zu können, und der Funktionsaufruf nimmt kaum weniger Platz ein als das Detail selbst.

Offensichtlich besteht ein Gleichgewicht zwischen Methoden, die zu viel und Methoden, die zu wenig leisten . Ich würde niemals eine winzige Funktion wie oben ausbrechen, es sei denn, sie würde von mehr als einer Stelle aus aufgerufen, und selbst dann würde ich zweimal darüber nachdenken, weil die Funktion hinsichtlich der Einführung neuer Logik und von as einfach nicht so wesentlich ist solche rechtfertigen kaum eine eigene Existenz.

Es scheint so, als ob Sie Folgendes tatsächlich wollen:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER

Dies sollte für jeden, der es liest, selbsterklärend sein: Setzen Sie es phoneNumberauf, resourceIdwenn es verfügbar ist, oder standardmäßig auf, DEV_PHONE_NUMBERwenn es nicht verfügbar ist .

Wenn Sie diese Variable wirklich nur in der Produktion festlegen möchten, sollten Sie eine andere, kanonischere, app-weite Dienstprogrammmethode verwenden (für die keine Parameter erforderlich sind), um zu bestimmen, von wo aus Sie ausgeführt werden. Das Lesen der Überschriften für diese Informationen macht keinen Sinn.

Lassen Sie mich ganz offen sein: Es scheint mir, dass Ihre Umgebung (Sprache / Framework / Klassendesign usw.) nicht wirklich für "sauberen" Code geeignet ist. Sie mischen alle möglichen Dinge in ein paar Codezeilen, die eigentlich nicht eng beieinander liegen sollten. Welches Geschäft hat eine einzelne Funktion mit dem Wissen, resourceId==undefdass Sie nicht in Produktion sind, dass Sie eine Standardtelefonnummer in Nicht-Produktionssystemen verwenden, dass die Ressourcen-ID in einigen "Headern" gespeichert ist und so weiter? Ich gehe davon aus, dass headerses sich um HTTP-Header handelt, sodass Sie die Entscheidung darüber, in welcher Umgebung Sie sich befinden, dem Endbenutzer überlassen.

Wenn Sie einzelne Teile davon in Funktionen zerlegen, können Sie das zugrunde liegende Problem kaum lösen.

Einige Schlüsselwörter zu suchen:

• Entkopplung
• Zusammenhalt
• Abhängigkeitsspritze

Sie können mit null Codezeilen erreichen, was Sie möchten (in anderen Kontexten), indem Sie die Verantwortlichkeiten des Codes verschieben und moderne Frameworks verwenden (die möglicherweise für Ihre Umgebung / Programmiersprache existieren oder nicht).

Nach Ihrer Beschreibung ("300 Codezeilen in einer 'Haupt'-Funktion") lässt mich sogar das Wort "Funktion" (anstelle von Methode) vermuten, dass es keinen Sinn macht, was Sie erreichen wollen. In dieser Programmierumgebung der alten Schule (dh grundlegende Imperativ-Programmierung mit wenig Struktur, sicherlich keine bedeutungsvollen Klassen, kein Klassen-Framework-Muster wie MVC oder ähnliches) ist es wirklich nicht sinnvoll, irgendetwas zu tun . Ohne grundlegende Veränderungen werden Sie niemals aus dem Sumpf herauskommen. Zumindest scheint es Ihnen Ihr Chef zu erlauben, Funktionen zu erstellen, um Code-Duplikate zu vermeiden, das ist ein guter erster Schritt!

Ich kenne sowohl den Typ des Codes als auch den Typ des Programmierers, den Sie recht gut beschreiben. Ehrlich gesagt, wenn es ein Mitarbeiter wäre, wäre mein Rat anders. Aber da es Ihr Chef ist, ist es für Sie nutzlos, darüber zu streiten. Nicht nur, dass Ihr Chef Sie außer Kraft setzen kann, sondern Ihre Code-Ergänzungen werden in der Tat zu schlechterem Code führen, wenn Sie nur "Ihr Ding" teilweise tun und Ihr Chef (und wahrscheinlich auch andere Leute) so weitermachen wie zuvor. Für das Endergebnis ist es möglicherweise besser, wenn Sie sich an den Programmierstil anpassen (natürlich nur während Sie an dieser speziellen Codebasis arbeiten) und in diesem Zusammenhang versuchen, das Beste daraus zu machen.

"Sauber" ist ein Ziel beim Schreiben von Code. Es ist nicht das einzige Ziel. Ein weiteres würdiges Ziel ist die Kolokalität . Informell ausgedrückt bedeutet Colocality, dass Leute, die versuchen, Ihren Code zu verstehen, nicht überall hüpfen müssen, um zu sehen, was Sie tun. Die Verwendung einer gut benannten Funktion anstelle eines ternären Ausdrucks scheint eine gute Sache zu sein, aber je nachdem, wie viele solcher Funktionen Sie haben und wo sie sich befinden, kann diese Praxis zu einem Ärgernis werden. Ich kann Ihnen nicht sagen, ob Sie diese Grenze überschritten haben, außer zu sagen, dass Sie zuhören sollten, wenn sich Leute beschweren, insbesondere, wenn diese Leute über Ihren Beschäftigungsstatus ein Mitspracherecht haben.

Die Verwendung kleiner Funktionen ist im Allgemeinen eine gute Praxis. Aber im Idealfall glaube ich, dass die Einführung einer Funktion entweder große logische Blöcke trennen oder die Gesamtgröße des Codes reduzieren sollte, indem man ihn austrocknet. Das Beispiel, das Sie beide gegeben haben, verlängert den Code und benötigt mehr Zeit, um von einem Entwickler gelesen zu werden, während die kurze Alternative nicht erklärt, dass der "resourceId"Wert nur in der Produktion vorhanden ist. So etwas Einfaches ist sowohl leicht zu vergessen als auch zu verwirren, wenn Sie versuchen, damit zu arbeiten, insbesondere, wenn Sie noch neu in der Codebasis sind.

Ich werde nicht sagen, dass Sie unbedingt eine Ternäre verwenden sollten, einige Leute, mit denen ich gearbeitet habe, bevorzugen die etwas längere if () {...} else {...}, es ist meistens eine persönliche Wahl. Ich bevorzuge normalerweise einen "One Line Do One Thing Approach", aber ich halte mich grundsätzlich an das, was in der Codebasis normalerweise verwendet wird.

Wenn bei Verwendung von ternary die logische Prüfung die Zeile zu lang oder zu kompliziert macht, sollten Sie erwägen, eine oder mehrere gut benannte Variable (n) zu erstellen, um den Wert zu speichern.

// NOTE "resourceId" not present in dev build, use test data
let isProduction = 'resourceId' in headers;
let phoneNumber = isProduction ? headers.resourceId : DEV_PHONE_NUMBER;

Ich möchte auch sagen, dass, wenn sich die Codebasis auf 300 Zeilenfunktionen ausdehnt, eine Unterteilung erforderlich ist. Aber ich würde die Verwendung von etwas breiteren Strichen empfehlen.

Das Codebeispiel, das Sie gegeben haben, ist Ihr Chef korrekt. Eine einzige klare Linie ist in diesem Fall besser.

Im Allgemeinen ist es für die Lesbarkeit, die Codewartung und die Möglichkeit, dass Unterklassen ein anderes Verhalten aufweisen (wenn auch nur geringfügig), besser, komplexe Logik in kleinere Teile zu zerlegen.

Ignorieren Sie nicht die Nachteile: Funktions-Overhead, Verschleierung (Funktion macht nicht, was Kommentare und Funktionsnamen implizieren), komplexe Spaghetti-Logik, das Potenzial für tote Funktionen (wurden zu einem Zeitpunkt für einen Zweck erstellt, der nicht mehr aufgerufen wird).

Ich kann mir mindestens zwei Argumente für lange Funktionen vorstellen:

• Es bedeutet, dass Sie viel Kontext um jede Zeile haben. Eine Möglichkeit, dies zu formalisieren: Zeichnen Sie das Kontrollflussdiagramm Ihres Codes. An einem Scheitelpunkt (~ = Linie) zwischen Funktionseingang und Funktionseingang kennen Sie alle eingehenden Kanten. Je länger die Funktion ist, desto mehr solche Eckpunkte gibt es.

• Viele kleine Funktionen bedeuten, dass es eine größere und komplexere Anrufgrafik gibt. Wähle eine zufällige Zeile in einer zufälligen Funktion und beantworte die Frage "In welchem ​​Kontext wird diese Zeile ausgeführt?" Dies wird schwieriger, je größer und komplexer das Aufrufdiagramm ist, da Sie mehr Eckpunkte in diesem Diagramm betrachten müssen.

Es gibt auch Argumente gegen lange Funktionen - man denke an die Testbarkeit von Einheiten. Nutzen Sie die Erfahrung, die Sie gemacht haben, wenn Sie zwischen beiden wählen.

Hinweis: Ich sage nicht, dass Ihr Chef Recht hat, nur, dass seine Perspektive möglicherweise nicht völlig wertlos ist.

Ich denke, meine Ansicht ist, dass der gute Optimierungsparameter nicht die Funktionslänge ist. Ich denke, eine nützlichere Desiderata ist die folgende: Wenn alles andere gleich ist, ist es vorzuziehen, aus dem Code eine allgemeine Beschreibung sowohl der Geschäftslogik als auch der Implementierung auszulesen. (Die Implementierungsdetails auf niedriger Ebene können immer gelesen werden, wenn Sie das entsprechende Codebit finden.)

Kommentar zu David Arnos Antwort :

Das Schreiben kleiner Funktionen ist ein Schmerz, da Sie gezwungen sind, in die einzelnen kleinen Funktionen zu wechseln, um zu sehen, was der Code tut.

Wenn die Funktion gut benannt ist, ist dies nicht der Fall. isApplicationInProduction ist selbstverständlich und es sollte nicht notwendig sein, den Code zu untersuchen, um zu sehen, was er tut. Tatsächlich ist das Gegenteil der Fall: Wenn Sie den Code untersuchen, sehen Sie weniger die Absicht als den Funktionsnamen (weshalb Ihr Chef auf Kommentare zurückgreifen muss).

Der Name macht deutlich, was der Rückgabewert bedeutet , sagt aber nichts über die Auswirkungen der Ausführung des Codes aus (= was der Code tut ). Namen vermitteln (nur) Informationen über Absichten , Code vermittelt Informationen über Verhaltensweisen (aus denen manchmal Teile der Absicht abgeleitet werden können).

Manchmal will man das eine, manchmal das andere, damit diese Beobachtung keine einseitig allgemeingültige Entscheidungsregel schafft.

Setzen Sie alles in eine große Hauptschleife, auch wenn die Hauptschleife mehr als 300 Zeilen umfasst, ist das Lesen schneller

Das Durchsuchen mag schneller sein, aber um den Code wirklich zu "lesen", müssen Sie ihn effektiv in Ihrem Kopf ausführen können. Das ist mit kleinen Funktionen einfach und mit Methoden, die Hunderte von Zeilen lang sind, wirklich sehr schwierig.

Ich bin damit einverstanden, dass Sie es in Ihrem Kopf ausführen müssen. Wenn Sie 500 Funktionszeilen in einer großen Funktion im Vergleich zu vielen kleinen Funktionen haben, ist mir nicht klar, warum dies einfacher wird.

Angenommen, der Extremfall von 500 Zeilen geradlinigem Code mit starken Nebeneffekten ist, und Sie möchten wissen, ob Effekt A vor oder nach Effekt B auftritt. Verwenden Sie im großen Funktionsfall Bild auf / ab, um zwei Zeilen zu lokalisieren und dann zu vergleichen Linien Nummern. In dem Fall mit vielen kleinen Funktionen müssen Sie sich merken, wo im Aufrufbaum die Effekte auftreten, und wenn Sie vergessen haben, dass Sie eine nicht unerhebliche Zeit damit verbringen müssen, die Struktur dieses Baums wiederzuentdecken.

Beim Durchlaufen des Aufrufbaums unterstützender Funktionen müssen Sie auch feststellen, wann Sie von der Geschäftslogik zu den Implementierungsdetails übergegangen sind. Ich behaupte ohne Beweise *, dass es umso einfacher ist, diese Unterscheidung zu treffen, je einfacher der Aufrufgraph ist.

(*) Zumindest bin ich ehrlich ;-)

Ich denke erneut, dass beide Ansätze Stärken und Schwächen haben.

Schreiben Sie nur kleine Funktionen, wenn Sie Code duplizieren müssen

Ich stimme dir nicht zu. Wie Ihr Codebeispiel zeigt, verbessern kleine, gut benannte Funktionen die Lesbarkeit von Code und sollten immer dann verwendet werden, wenn [zB] Sie sich nicht für das "Wie", sondern nur für das "Was" einer Funktion interessieren.

Ob Sie sich für das "Wie" oder "Was" interessieren, hängt von dem Zweck ab, für den Sie den Code lesen (z. B. eine allgemeine Vorstellung davon zu bekommen oder einen Fehler aufzuspüren). Der Zweck, für den Sie den Code lesen, ist beim Schreiben des Programms nicht verfügbar, und Sie werden den Code höchstwahrscheinlich für verschiedene Zwecke lesen. Unterschiedliche Entscheidungen werden für unterschiedliche Zwecke optimiert.

Dies ist jedoch der Teil der Ansicht des Chefs, mit dem ich wahrscheinlich am wenigsten einverstanden bin.

Schreiben Sie keine Funktion mit dem Namen des Kommentars, sondern setzen Sie Ihre komplexe Codezeile (3-4 Zeilen) mit einem Kommentar darüber. Auf diese Weise können Sie den fehlerhaften Code direkt ändern

Ich kann die Gründe für diese wirklich nicht verstehen, vorausgesetzt, es ist wirklich ernst. [...] Kommentare haben einen fundamentalen Fehler: Sie werden nicht kompiliert / interpretiert und können daher nicht in der Einheit getestet werden. Der Code wird geändert und der Kommentar wird in Ruhe gelassen und Sie wissen am Ende nicht, welcher richtig ist.

Compiler vergleichen nur Namen auf Gleichheit. Sie geben Ihnen niemals einen irreführenden Namensfehler. Da mehrere Aufrufstellen eine bestimmte Funktion nach Namen aufrufen können, ist es manchmal schwieriger und fehleranfälliger, einen Namen zu ändern. Kommentare haben dieses Problem nicht. Dies ist jedoch etwas spekulativ; Um dies wirklich zu regeln, würde man wahrscheinlich Daten darüber benötigen, ob Programmierer eher irreführende Kommentare als irreführende Namen aktualisieren, und das habe ich nicht.

Meiner Meinung nach ist der richtige Code für die Funktionalität, die Sie benötigen:

phoneNumber = headers.resourceId || DEV_PHONE_NUMBER;

Oder wenn Sie es in eine Funktion aufteilen möchten, wahrscheinlich so etwas wie:

phoneNumber = getPhoneNumber(headers);

function getPhoneNumber(headers) {
  return headers.resourceId || DEV_PHONE_NUMBER
}

Aber ich denke, Sie haben ein grundlegenderes Problem mit dem Konzept "in Produktion". Das Problem mit Ihrer Funktion isApplicationInProductionist, dass es seltsam erscheint, dass dies der einzige Ort im System ist, an dem es auf "Produktion" ankommt, und dass Sie sich immer darauf verlassen können, dass der Header resourceId vorhanden oder nicht vorhanden ist. Es sollte eine allgemeine isApplicationInProductionMethode oder getEnvironmentMethode geben, die die Umgebung direkt überprüft. Der Code sollte wie folgt aussehen:

function isApplicationInProduction() {
  process.env.NODE_ENV === 'production';
}

Dann können Sie die Telefonnummer erhalten mit:

phoneNumber = isApplicationInProduction() ? headers.resourceId : DEV_PHONE_NUMBER;

Nur ein Kommentar zu zwei der Stichpunkte

• Das Schreiben kleiner Funktionen ist ein Schmerz, da Sie gezwungen sind, in jede kleine Funktion zu wechseln, um zu sehen, was der Code tut.
• Setzen Sie alles in eine große Hauptschleife, auch wenn die Hauptschleife mehr als 300 Zeilen umfasst, ist das Lesen schneller.

In vielen Editoren (z. B. IntelliJ) können Sie zu einer Funktion / Klasse springen, indem Sie bei gedrückter Strg-Taste auf die Verwendung klicken. Außerdem müssen Sie häufig die Implementierungsdetails einer Funktion nicht kennen, um den Code zu lesen, wodurch das Lesen des Codes beschleunigt wird.

Ich empfehle Ihnen, es Ihrem Chef zu sagen. er wird Ihr Eintreten mögen und es als Führung sehen. Sei einfach höflich.