Warum werden andere Anweisungen in Ruby nicht empfohlen?

Question

Ich war neulich auf der Suche nach einem Ruby-Code-Tool, und ich stieß auf den Edelstein pelusa, der interessant aussieht. Eines der Dinge, nach denen es sucht, ist die Anzahl der else-Anweisungen, die in einer gegebenen Ruby-Datei verwendet werden.

Meine Frage ist, warum sind diese schlecht? Ich verstehe, dass if/else Aussagen oft eine große Komplexität hinzufügen (und ich verstehe, dass das Ziel ist, die Komplexität des Codes zu reduzieren), aber wie kann eine Methode, die zwei Fälle überprüft, ohne eine else geschrieben werden?

Zur Erinnerung, ich habe zwei Fragen:

1) Gibt es einen anderen Grund als die Komplexität des Codes zu reduzieren, dass andere Aussagen vermieden werden könnten?

2) Hier ist eine Beispiel-Methode aus der App, die ich arbeite, die eine Anweisung verwendet. Wie würdest du das ohne einen schreiben? Die einzige Option, die mir einfallen würde, wäre eine ternäre Aussage, aber hier ist genug Logik, dass eine ternäre Aussage tatsächlich komplexer und schwieriger zu lesen wäre.

def deliver_email_verification_instructions 
    if Rails.env.test? || Rails.env.development? 
    deliver_email_verification_instructions! 
    else 
    delay.deliver_email_verification_instructions! 
    end 
end 

Wenn Sie dies mit einem ternären Operator schreiben, wäre es:

def deliver_email_verification_instructions 
    (Rails.env.test? || Rails.env.development?) ? deliver_email_verification_instructions! : delay.deliver_email_verification_instructions! 
end 

Ist das richtig? Wenn ja, ist das nicht schwerer zu lesen? Ist eine else Anweisung nicht hilfreich? Gibt es einen anderen, besseren, else - keine Möglichkeit, dies zu schreiben, an die ich nicht denke?

Ich denke, ich suche hier nach stilistischen Überlegungen.

Answer 1

mich beginnen lassen, indem er sagte, dass es nicht wirklich etwas falsch mit Ihrem Code ist, und in der Regel sollten Sie sich bewusst sein, dass das, was ein Code-Qualität Werkzeug sagt Ihnen könnte völliger Unsinn, denn es fehlt der Kontext, um zu bewerten, was Sie tatsächlich tun.

Aber zurück zum Code. Wenn es eine Klasse war, die genau eine Methode hatte, wo das Snippet

if Rails.env.test? || Rails.env.development? 
    # Do stuff 
else 
    # Do other stuff 
end 

aufgetreten, die völlig in Ordnung sein würde (es gibt immer verschiedene Ansätze zu einer bestimmten Sache, aber Sie brauchen keine Sorgen zu machen, auch wenn Programmierer werden dich hassen, dass du nicht mit ihnen darüber gestritten hast: D).

Jetzt kommt der schwierige Teil. Die Leute sind faul wie die Hölle, und so sind Codeschnipsel wie die oben genannten einfache Ziele für das Kopieren/Einfügen-Codieren (deshalb werden Leute argumentieren, dass man sie überhaupt vermeiden sollte, denn wenn du später eine Klasse erweiterst, bist du wahrscheinlicher einfach Zeug kopieren und einfügen, als es tatsächlich zu refaktorieren).

Schauen wir uns Ihr Code-Snippet als Beispiel an. Ich schlage im Prinzip dasselbe vor wie @Mik_Die, aber sein Beispiel ist genauso anfällig dafür, kopiert/eingefügt zu werden. Daher ist dies getan (IMO) sollte würde:

class Foo 
    def initialize 
    @target = (Rails.env.test? || Rails.env.development?) ? self : delay 
    end 

    def deliver_email_verification_instructions 
    @target.deliver_email_verification_instructions! 
    end 
end 

Dies könnte nicht wie es ist, um Ihre Anwendung anwendbar sein, aber ich hoffe, dass Sie auf die Idee kommen, das ist: Setzen Sie sich nicht wiederholen. Je.Jedes Mal, wenn Sie sich wiederholen, machen Sie Ihren Code nicht nur weniger wartbar, sondern dadurch auch fehleranfälliger für die Zukunft, weil ein oder sogar 99/100 Vorkommnisse von allem, was Sie kopiert und eingefügt haben, geändert werden können das eine verbleibende Vorkommen ist, was die @disasterOfEpicProportions am Ende :) verursacht

---

Ein weiterer Punkt, dass ich vergessen habe wurde von @RayToal (danke :), die gebracht ist, dass wenn/else-Konstrukte werden häufig verwendet, in Kombination mit booleschen Eingabeparametern, die zu Konstrukten wie diesem führen (tatsächlicher Code aus einem Projekt, den ich pflegen muss):

class String 
    def uc(only_first=false) 
    if only_first 
     capitalize 
    else 
     upcase 
    end 
    end 
end 

Lassen Sie uns die offensichtliche Methodenbenennung und Affepatching-Probleme hier ignorieren und konzentrieren Sie sich auf das If/Else-Konstrukt, das verwendet wird, um der uc Methode zwei verschiedene Verhaltensweisen abhängig von dem Parameter only_first zu geben. Ein solcher Code ist eine Verletzung des Grundsatzes der einfachen Verantwortlichkeit, weil Ihre Methode mehr als eine Sache tut, weshalb Sie zwei Methoden an erster Stelle geschrieben haben sollten.

Answer 2

def deliver_email_verification_instructions 
    subj = (Rails.env.test? || Rails.env.development?) ? self : delay 
    subj.deliver_email_verification_instructions! 
end