Refactoring: due grandi blocchi all'interno di una funzione

Se non vuoi iniziare con la cosa ovvia, puoi anche iniziare con l'identificazione di piccole parti comuni nel blocco A e B e rifattorizzare ciascuna parte in un metodo separato che viene quindi chiamato da A e da B Idealmente, questo ti porterà ai nuovi blocchi A e B che contengono solo funzionalità non comuni.

Ma spostare queste funzionalità in metodi separati probabilmente sarà comunque una buona idea, e il risultato finale sarà lo stesso di quello che avresti fatto al contrario.

Secondo la mia esperienza, l'estrazione di piccole funzioni comuni da A e B ha il vantaggio di poter gestire piccoli metodi che sono spesso più facili da comprendere, quindi la possibilità di introdurre errori è minore. Facendo questo step-by-step si riduce anche la dimensione dei blocchi A e B, e si aumenta la loro leggibilità, quindi in seguito il refactoring di A e B ai metodi diventa più facile.

Il modo ovvio è un buon modo per iniziare. Una volta che hai definito i metodi aeb, puoi estrarre la loro comunanza in c - a quel punto forse puoi re-inline a e b nel metodo originale.

Puoi modellare la tua condizione "se" come due classi diverse. Presumo che ci siano 3 differenze tra i tuoi "grossi blocchi di dichiarazioni". Potresti chiamare OneWay e l'altro AnotherWay.

Class OneWay(ParentWay):
    myWay(a, b, c):
        #Large block of statements.

Class AnotherWay(ParentWay):
    myWay(a, b, c):
         #another large block of statements.

Ecco come useresti queste due classi

do_something(someWay):
    someWay.myWay(a, b, c)

Ora farà la cosa giusta in base a quale tipo di classe è passato. Questo tipo di rovesciamento delle cose.

Non sto dicendo che un modo è meglio dell'altro, ma dipende dalla situazione. Sicuramente se l'intero metodo è un enunciato big if / else e if-block e else-blocks sono quasi identici, allora è probabilmente richiesto un tipo di refactoring.

Una soluzione più semplice e probabilmente migliore è questa:

do_true_thing():
     #Large block of statements.

do_false_thing():
     #another large block of statements.

Quindi, invece di passare un vero / falso, il client chiama un metodo appropriato. Useresti nomi migliori. In generale, non mi piace avere booleani nelle firme dei metodi, a meno che non sia davvero ovvio cosa intendano. Quando ottieni un metodo come do_stuff(isTrue, isReallyTrue) ho difficoltà a ricordare cosa significano i parametri.

Usando una di queste tecniche, potresti comunque voler rompere i grandi blocchi in diverse procedure come suggerito da @CarlManaster.

Perdonate il mio pseudocodice, davvero non conosco Python, ma i concetti dovrebbero ancora essere applicati.

Per prima cosa, ci sono già test automatici su questo codice? Assumendo no, c'è un modo per scrivere un test automatizzato attorno a questo codice per assicurarti di non romperlo quando lo rifatti? Probabilmente risparmierai tempo a lungo termine se lo fai prima.

Dopo averlo fatto, farei ciò che @CarlManaster. Estraili in due metodi e pezzo per pezzo estrai le parti che hanno in comune in un unico metodo che entrambi condividono. Può essere d'aiuto se hai modo di confrontare i due metodi con uno strumento diff. Uso intellij-idea e lo rende piuttosto facile da fare.

puoi spostare i 2 blocchi in funzioni separate e utilizzare un semplice dispatcher:

def do_something():
    function_1() if isTrue else function_2()

1. Questo impedirà di avere 2 "brutti" blocchi di codice
2. È una cattiva idea trattare le funzioni come "sacche di codice" - se il codice fa differenza e poi separarle.
3. Renderà il codice più leggibile e pietoso - futuro tu ei maintainer ti ringrazieranno

Ho visto un codice come questo in passato, con "il grande blocco di dichiarazioni" costituito da molte centinaia di righe di codice.

Per cominciare, qualsiasi "grosso blocco di dichiarazioni" è, di per sé, un odore di codice. Come alcuni degli altri poster hanno menzionato, è necessario suddividerlo in pezzi più piccoli di codice.

Per quanto riguarda la gestione delle differenze, non c'è vergogna nell'usare più blocchi if-else nelle stesse condizioni. Scrivere qualcosa come:

do_something():
    if(condition):
        do_stuff()
    else:
        do_other():
    # shared code
    if(condition):
        make_thing()
    else:
        make_other()
    # shared code
    if(condition):
        return this_stuff()
    else:
        return that_stuff()

Se sei fortunato, le differenze si riducono a poche costanti e puoi pulire questo casino scrivendo

do_something():
    if(condition):
        a = 10
        b = -1
        c = 1.5
    else:
        a = 100
        b = +1
        c = 1.9
    # shared code

Stai dicendo che i due pezzi "fanno la stessa cosa ma con qualche leggera variazione". Le variazioni sono abbastanza piccole da poter sostituire ciascuno dei blocchi con la funzione la stessa , controllata da argomenti leggermente diversi? Non farlo se creerebbe un codice ancora più brutto con un sacco di test if / then, ovviamente; ma se i due blocchi "fanno la stessa cosa", pensa a ciò che hanno in comune e prova a farlo apparire in una singola funzione.

Da un punto di vista OO, ci sono due schemi che potrebbero essere appropriati in questo caso; Modello e Strategia .

Il modello Template consente di modificare i passaggi del proprio algoritmo in fase di esecuzione. Il modello di strategia ti consente di modificare l'algoritmo.

Il tuo pseudo codice assomiglia un po 'a Python e l'implementazione dei pattern varia a seconda degli idiomi della lingua.

Ecco un esempio di strategia:

class Context:
    def __init__(self, strategy):
        self.strategy = strategy
    def DoSomething(self):
        self.strategy.DoSomething();

class Strategy:
    def DoSomething(self):
        raise NotImplementedError # You have to override.

class StrategyA(Strategy):
    def DoSomething(self):
        #Large block of statements.
        print "Evaluate StrategyA"

class StrategyB(Strategy):
    def DoSomething(self):
        #another large block of statements.
        print "Evaluate StrategyB"

c = Context(StrategyA())
c.DoSomething();

c = Context(StrategyB())
c.DoSomething();

Modello modello:

class Context:
    def __init__(self, template):
        self.template = template
    def DoSomething(self):
        self.template.Step1(); # represents a difference between the two algorithms
        # common steps in here...
        self.template.Step2(); # difference...
        # another common step here...
        self.template.Step3(); # difference...
        self.template.Step4();

class Template:
    def Step1(self):
        raise NotImplementedError # You have to override.
    def Step2(self):
        raise NotImplementedError # You have to override.
    def Step3(self):
        raise NotImplementedError # You have to override.
    def Step4(self):
        raise NotImplementedError # You have to override.

class TemplateA(Template):
    def Step1(self):
        print "Step1A"
    def Step2(self):
        print "Step2A"
    def Step3(self):
        print "Step3A"
    def Step4(self):
        print "Step4A"

class TemplateB(Template):
    def Step1(self):
        print "Step1B"
    def Step2(self):
        print "Step2B"
    def Step3(self):
        print "Step3B"
    def Step4(self):
        print "Step4B"

c = Context(TemplateA())
c.DoSomething();

c = Context(TemplateB())
c.DoSomething();

Tuttavia, senza più contesto, se è difficile dire se questo è il migliore in questa situazione. Entrambi i modelli potrebbero esagerare per soli due passaggi. Potrebbe essere meglio suddividere il tuo metodo in metodi separati.