Perché le altre affermazioni sono scoraggiate in Ruby?

Question

L'altro giorno stavo cercando uno strumento di qualità del codice Ruby e mi sono imbattuto nella gemma pelusa, che sembra interessante. Una delle cose che controlla è il numero di altre istruzioni usate in un dato file Ruby.

La mia domanda è, perché sono così male? Capisco che le dichiarazioni if/else aggiungano spesso una grande quantità di complessità (e ho capito che l'obiettivo è ridurre la complessità del codice), ma come può un metodo che verifica che due casi vengano scritti senza un else?

Per ricapitolare, ho due domande:

1) C'è un motivo diverso da ridurre la complessità del codice che else potrebbero essere evitati?

2) Ecco un metodo di esempio dall'app su cui sto lavorando che utilizza un'istruzione else. Come lo scriverebbe senza uno? L'unica opzione che potrei pensare sarebbe una dichiarazione ternaria, ma qui c'è abbastanza logica che penso che una dichiarazione ternaria sarebbe in realtà più complessa e più difficile da leggere.

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

Se hai scritto questo con un operatore ternario, sarebbe:

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

È quello giusto? Se è così, non è così difficile da leggere? Una dichiarazione di else non aiuta a risolvere questo problema? C'è un altro, meglio, else - modo in cui scrivere questo a cui non sto pensando?

Immagino di dover cercare considerazioni stilistiche qui.

Answer 1

Permettetemi di cominciare dicendo che non c'è davvero nulla di sbagliato con il codice, e in genere si dovrebbe essere consapevoli del fatto che tutto ciò che uno strumento di qualità del codice dice che potrebbe essere assolutamente privi di senso, perché manca il contesto per valutare ciò che stai effettivamente facendo.

Ma torniamo al codice. Se ci fosse una classe che aveva esattamente un metodo in cui si è verificato il frammento di

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

, che sarebbe completamente bene (ci sono sempre diversi approcci per una determinata cosa, ma non è necessario preoccuparsi di questo, anche se i programmatori ti odieranno per non litigare con loro al riguardo: D).

Ora arriva la parte difficile. Le persone sono pigre come l'inferno, e quindi frammenti di codice come quello sopra sono facili bersagli per la codifica di copia/incolla (questo è il motivo per cui le persone sosterranno che in primo luogo si dovrebbero evitare, perché se si espande una classe in un secondo momento è più probabile semplicemente copiare e incollare roba piuttosto che rifattarla effettivamente).

Diamo un'occhiata al codice snippet come esempio. Fondamentalmente sto proponendo la stessa cosa di @Mik_Die, tuttavia il suo esempio è ugualmente incline a essere copia/incollato come il tuo. Pertanto, sarebbe dovrebbe essere fatto (IMO) è questo:

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 

questo potrebbe non essere applicabile per la vostra applicazione, come è, ma spero che si ottiene l'idea, che è: Non ripetere te stesso. Mai.Ogni volta che ripeti te stesso, non solo stai rendendo il tuo codice meno gestibile, ma di conseguenza anche più soggetto a errori in futuro, perché potrebbero essere cambiate una o anche 99/100 occorrenze di qualunque cosa tu abbia copiato e incollato, ma l'un'occorrenza rimanente è ciò che provoca il @disasterOfEpicProportions alla fine :)

---

un altro punto che ho dimenticato è stato portato da @RayToal (grazie :), che è che se/costrutti else sono spesso utilizzati in combinazione con parametri di ingresso booleani, con conseguente costrutti come questa (codice effettivo da un progetto devo mantenere):

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

Cerchiamo di ignorare l'ovvio problema di denominazione dei metodi e patch di scimmia qui, e ci concentriamo sul costrutto if/else, che è usato per dare al metodo uc due comportamenti diversi a seconda del parametro only_first. Tale codice è una violazione del principio di responsabilità unica, perché il tuo metodo è facendo più di una cosa, motivo per cui dovresti aver scritto due metodi in primo luogo.

Answer 2

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