Ist es sinnvoll, Code in der Hoffnung auf eine Qualitätsverbesserung im Miniformat umzugestalten, oder wird nur Code ohne großen Nutzen verschoben?

12

Beispiel

Ich bin auf monolithischen Code gestoßen, der "alles" an einem Ort erledigt - Daten aus der Datenbank laden, HTML-Markup anzeigen und als Router / Controller / Aktion fungieren. Ich begann SRP anzuwenden, um Datenbankcode in eine eigene Datei zu verschieben, um bessere Namen für die Dinge zu erhalten, und alles sah gut aus, aber dann begann ich Zweifel zu haben, warum ich das tue.

Warum umgestalten? Was ist der Zweck? Ist es nutzlos Was ist der vorteil Beachten Sie, dass ich die monolithische Datei größtenteils so belassen habe, wie sie ist, aber nur den kleineren Teil überarbeitet habe, der für den Bereich relevant war, in dem ich etwas arbeiten musste.

Originalcode:

Um ein konkretes Beispiel zu geben, bin ich auf dieses Code-Snippet gestoßen - es lädt Produktspezifikationen entweder über eine bekannte Produkt-ID oder über eine vom Benutzer ausgewählte Versions-ID:

if ($verid)
    $sql1 = "SELECT * FROM product_spec WHERE id = " . clean_input($verid);
else
    $sql1 = "SELECT * FROM product_spec WHERE product_id = " . clean_input($productid) ;
$result1 = query($sql1);
$row1 = fetch_array($result1);
/* html markup follows */

Refactoring:

Da ich einige Arbeiten erledige, bei denen ich Änderungen an diesem bestimmten Teil des Codes vornehmen muss, habe ich das Repository-Muster geändert und es aktualisiert, um objektorientierte MySQL-Funktionen zu verwenden:

//some implementation details omitted 
$this->repository = new SpecRepository($mysql);
if ($verid)
    $row1 = $this->repository->getSpecByVersion($verid);
else 
    $row1 = $this->repository->getSpecByProductId($productid);
/* html markup follows to be refactored or left alone till another time*/

//added new class:
class SpecRepository extends MySqlRepository
{

    function getSpecByVersion(int $verid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE id = ?
        ", $verid)->getSingleArray();
    }

    function getSpecByProductId(int $productid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE product_id = ?
        ", $productid)->getSingleArray();
    }
}

Soll ich das machen

Rückblickend auf die Änderungen ist der Code immer noch vorhanden, Code mit derselben Funktionalität, aber in unterschiedlichen Dateien, unterschiedlichen Namen und Orten, wobei eher ein objektorientierter Stil als ein prozeduraler verwendet wird. Eigentlich ist es lustig zu bemerken, dass der überarbeitete Code trotz der gleichen Funktionalität viel aufgeblähter aussieht.

Ich sehe einige Antworten mit den Worten "Wenn Sie nicht wissen, warum Sie umgestalten, tun Sie es nicht", und vielleicht stimme ich Ihnen zu. Meine Gründe sind, die Qualität des Codes im Laufe der Zeit zu verbessern (und ich hoffe, dass ich dies tun werde, indem ich SRP und andere Prinzipien befolge).

Sind das gute Gründe oder verschwende ich meine Zeit damit, Code auf diese Weise neu zu ordnen? Um ehrlich zu sein, fühlt sich das Umgestalten insgesamt ein bisschen wie das Treten von Wasser an - es braucht Zeit und wird in Bezug auf SRP "getrennter", aber trotz meiner guten Absichten habe ich nicht das Gefühl, dass ich erstaunliche Verbesserungen mache. Daher wird überlegt, ob es am besten ist, den Code wie zuvor zu belassen und nicht umzugestalten.

Warum habe ich überhaupt umgestaltet?

In meinem Fall füge ich einer neuen Produktlinie neue Funktionen hinzu, sodass ich entweder der vorhandenen Codestruktur für ähnliche Produktlinien folgen oder meine eigene schreiben muss.

Dennis
quelle
OT aber etwas zu berücksichtigen ist, dass Select * from ..ein Anti-Muster betrachtet werden kann. Siehe stackoverflow.com/q/3639861/31326
Peter M
1
@PeterM: Wenn Sie kein ORM schreiben, select *wird dies zu einer "Best Practice".
Robert Harvey
@RobertHarvey In diesem Fall frage ich mich, warum Sie Ihr eigenes ORM schreiben: D
Peter M
Ich habe Refaktoren für irrelevantere Ursachen gemacht. Der Abbau der technischen Schulden ist gut, sobald der Nutzen die Kosten übersteigt. Sieht so aus, als wäre das dein Fall. Die Frage ist: Haben Sie die richtigen Tests, um sicherzustellen, dass der Code weiterhin wie erwartet funktioniert?
Laiv
1
Zuverlässigkeit sollte die vorherrschende Messgröße beim Refactoring von IMO sein, nicht die Wartbarkeit, die sich auf die Änderbarkeit bezieht, da Zuverlässigkeit die Gründe für Änderungen in erster Linie verringert. Es verbessert die Stabilität, und Code, der perfekt stabil und zuverlässig ist und alles Notwendige erfüllt, verursacht nur geringe Wartungskosten, da es nur sehr wenige Gründe gibt, sich jemals ändern zu müssen. Versuchen Sie, die Gründe für Änderungen zu reduzieren, anstatt zu versuchen, sie so einfach wie möglich zu gestalten. Das Suchen des ersteren wird auch dazu neigen, etwas von dem letzteren zu geben.

Antworten:

13

In dem konkreten Beispiel, das Sie angegeben haben, war ein Refactoring möglicherweise nicht erforderlich, zumindest noch nicht. Aber Sie haben gesehen, dass es in Zukunft ein Potenzial für chaotischeren Code gibt, der zu einem längeren und langwierigeren Refactoring geführt hätte. Sie haben also die Initiative ergriffen und die Dinge jetzt aufgeräumt.

Ich würde auch sagen, dass der Vorteil, den Sie hier gewonnen haben, Code ist, der zumindest aus meiner Sicht einfacher zu verstehen ist, als jemand, der Ihre Codebasis nicht kennt.


Allgemein:

Erleichtert das Refactoring anderen Entwicklern das Verständnis Ihres Codes?

Erleichtert das Refactoring die Verbesserung des Codes?

Erleichtert das Refactoring die Pflege des Codes?

Erleichtert das Refactoring das Debuggen?

Wenn die Antwort auf eine dieser Fragen "Ja" war, dann war es das wert und es war mehr als nur "Code bewegen"?

Wenn die Antwort auf alle diese Fragen "Nein" war, musste sie möglicherweise nicht überarbeitet werden.

Die endgültige Größe der Codebasis könnte in Bezug auf die LoC größer sein (obwohl einige Refactorings die Codebasis durch Straffung von redundantem Code verkleinern können), aber das sollte kein Faktor sein, wenn es andere Vorteile gibt.

FrustratedWithFormsDesigner
quelle
9

Refactoring bläht sich auf, wenn Sie einen Monolithen zerlegen.

Sie haben den Vorteil jedoch bereits gefunden.

"In meinem Fall füge ich einer neuen Produktlinie neue Funktionen hinzu."

Sie können einem Monolithen nicht ohne weiteres Funktionen hinzufügen, ohne Dinge zu beschädigen.

Sie sagten, dass derselbe Code die Daten abruft und das Markup ausführt. Großartig, bis Sie ändern möchten, welche Spalten Sie auswählen und bearbeiten, um sie unter bestimmten Bedingungen anzuzeigen. Plötzlich funktioniert Ihr Markup-Code in einem bestimmten Fall nicht mehr und Sie müssen umgestalten.

Diakon
quelle
Ja, aber ich kann dem Monolithen auch weiterhin neue Funktionen hinzufügen. Ich würde also einen neuen if/elseBlock hinzufügen und dem ursprünglichen Codestil folgen, um SQL-Anweisungen hinzuzufügen, und dann ein neues HTML-Markup in dieselbe Monolithdatei einfügen. Und um die Spalten zu ändern, würde ich den if / else-Block finden, der für die Unterbringung von SQL-Zeilen verantwortlich ist, und diese SQL bearbeiten, um Änderungen an den Spalten vorzunehmen, ohne diese Datei aufzulösen.
Dennis
Bei einem überarbeiteten Ansatz würde ich wahrscheinlich zuerst zum if / else-Block im Monolithen gehen, um festzustellen, dass ich zur separaten Repository-Datei wechseln muss, um SQL zu ändern. Oder wenn ich kürzlich an diesem Monolithen gearbeitet hätte, hätte ich gewusst, dass ich stattdessen direkt in die Repository-Datei gehen und den Monolithen umgehen würde. Oder wenn ich meine Codebasis nach der spezifischen SQL durchsucht hätte, würde mich die Suche in die neue Repository-Datei versetzen und auch den Monolithen umgehen
Dennis
2

Ich überlege mir eine Umgestaltung, wenn ich weiß, dass ich Monate oder sogar Jahre mit diesem Projekt arbeiten muss. Wenn ich hier und da eine Änderung vornehmen muss, dann widersetze ich mich dem Drang, Code zu verschieben.

Die bevorzugte Methode zum Refactoring ist, wenn ich einen Basiscode mit einer Art automatisierten Tests habe. Trotzdem muss ich sehr vorsichtig sein, was ich ändere, um sicherzustellen, dass die Dinge wie zuvor funktionieren. Sie werden sehr bald das Vertrauen verlieren, wenn Sie Fehler in anderen Bereichen als dem, in dem Sie arbeiten sollten, melden.

Ich schätze Refactoring aus folgenden Gründen:

  • Ich verstehe den Code besser
  • Ich entferne doppelten Code. Wenn ich also einen Fehler an einer Stelle habe, muss ich nicht an jeder Stelle suchen, an der der Code kopiert wurde
  • Ich erstelle Klassen, deren Rolle klar definiert ist. Zum Beispiel kann ich dasselbe Repository wiederverwenden, um eine HTML-Seite, einen XML-Feed oder einen Hintergrundjob zu hydratisieren. Dies wäre nicht möglich gewesen, wenn der Datenbankzugriffscode nur in der HTML-Klasse enthalten wäre
  • Ich benenne Variablen, Methoden, Klassen, Module, Ordner um, wenn sie nicht mit der aktuellen Realität übereinstimmen. Ich habe commentden Code über Variablennamen, Methodennamen
  • Ich löse komplexe Fehler mit einer Kombination aus Unit-Tests und Refactoring
  • Ich habe die Flexibilität, ganze Module, Pakete zu ersetzen. Wenn der aktuelle ORM veraltet, fehlerhaft oder nicht mehr gewartet ist, kann er leichter ersetzt werden, wenn er nicht über das gesamte Projekt verteilt ist.
  • Ich entdecke neue Funktionen oder Verbesserungen, die zu Vorschlägen für den Kunden führen.
  • Ich habe wiederverwendbare Komponenten erstellt. Dies war einer der größten Vorteile, da die in diesem Projekt geleistete Arbeit möglicherweise für einen anderen Kunden / ein anderes Projekt wiederverwendet wurde.
  • Ich beginne oft mit der dümmsten, hardcodierten, verrückten Lösung und überarbeite sie später, sobald ich mehr erfahre. Dieser spätere Moment könnte heute oder vielleicht nach mehreren Tagen sein. Ich möchte nicht zu viel Zeit damit verbringenarchitect die Lösung aufwenden. Dies wird beim Schreiben passieren - den Code hin und her schreiben.

In einer perfekten Welt sollten Refactorings durch Unit-Tests, Integrationstests usw. verifiziert werden. Wenn es keine gibt, füge sie hinzu. Auch einige IDE könnte viel helfen. Normalerweise benutze ich ein Plugin, das Geld kostet. Ich möchte wenig Zeit mit Refactorings verbringen und dabei sehr effizient sein.

Ich habe auch Bugs eingeführt. Ich kann sehen, warum einige QA fragenare you guys doing refactoring? because everything stopped working! Dies ist das Risiko, das das Team eingehen muss, und wir müssen immer transparent darüber sein.

Im Laufe der Jahre stellte ich fest, dass kontinuierliches Refactoring meine Produktivität verbesserte. Es war viel einfacher, neue Funktionen hinzuzufügen. Auch große Umbauten erforderten kein vollständiges Umschreiben, wie dies häufig der Fall war, wenn die Codebasis nicht an die Produktentwicklung angepasst wurde. Es macht auch Spaß.

Adrian Iftode
quelle
Dies steht kurz vor einer Abwärtsabstimmung - Es gibt kein "Ich" in "Computerprogrammierer". Mit "Ich" meinst du mich "Ich" (Abgestimmt) oder meinst du die Entwickler, die jetzt und in Zukunft am Code arbeiten?
Mattnz
Mir ist es auch aufgefallen, aber ich habe es formuliert. Außerdem habe ich viele Jahre als Solo-Entwickler gearbeitet und als ich diese Antwort schrieb, wiederholte ich hauptsächlich diese Erfahrung. Ich konnte ersetzen Imit tothough.
Adrian Iftode
1

Ich denke, die Frage, die Sie sich stellen müssen, ist nicht so sehr "Gibt es einen Nutzen?" aber "Wer wird für diese Arbeit bezahlen?"

Wir können uns alle über die wahrgenommenen Vorteile bis zum Ende der Tage streiten, aber wenn Sie keinen Kunden haben, der an diese Vorteile glaubt und sie genug schätzt, um für die Änderungen zu zahlen, sollten Sie sie nicht tun.

Es ist allzu einfach, wenn Sie irgendwo im Entwicklerteam sind, um Code zu sehen und ihn umzugestalten, um ihn "besser" zu machen. Es ist jedoch sehr schwierig, einen Wert auf "besseren Code" zu setzen, wenn das Produkt bereits funktioniert.

Dinge wie die "schnellere Entwicklung neuer Funktionen" schränken es nicht ein, weil sie nur die Kosten senken. Sie müssen Umsatz generieren.

Ewan
quelle
1

Meiner Meinung nach ist Refactoring eine gute Investition.

Andernfalls erhalten Sie bald technische Schulden (ungelöste Probleme, fehlerhafter Code, der nur unter bestimmten Bedingungen funktioniert, usw.). Die technischen Schulden werden bald immer größer, bis die Software nicht mehr wartbar ist.

Damit das Refactoring funktioniert, müssen Sie jedoch auch in Tests investieren. Sie sollten automatisierte Tests erstellen. Idealerweise sollten Sie Komponententests und Integrationstests durchführen. Dies verhindert, dass Sie vorhandenen Code beschädigen.

Wenn Sie Ihren Chef oder Kollegen überzeugen müssen, sollten Sie einige Bücher über agile Methoden (zB SCRUM) und testgetriebene Entwicklung lesen.

Bernie
quelle