Refactoring di metodi lunghi con molta complessità ciclomatica

2

Sto tentando di ridefinire quello che sta diventando un metodo molto grande - attualmente circa 350 linee - che contiene un alto grado di complessità ciclomatica .

Capisco e attribuisco alle teorie che i metodi dovrebbero essere brevi e che le preoccupazioni dovrebbero essere separate, e ho letto cose come this e questo tra gli altri. Sto lottando per rifattorizzare il mio metodo, però, per un paio di ragioni. Per aiutare a illustrare questi, prima ecco un esempio semplificato di come appare il metodo prima del refactoring:

private static void ProcessMessage(Message toProcess)
{
    var processingIssues = new StringBuilder();
    var transformedAttachments = new List<TransformedAttachmentData>();

    foreach (var attach in toProcess.Attachments)
    {
        //in reality there is quite a bit of nested logic here,
        //but I'll represent it as a single if-else block.
        if (true)
        {
            //in reality, a third-party library is used in multiple
            //levels of this logic hierarchy to produce the transformed data.
            transformedAttachments.Add(new TransformedAttachmentData(attach));
        }
        else
        {
            processingIssues.AppendLine(
                string.Format(@"Issue ""{0}"" occurred with " +
                @"attachment ""{1}"".", "blah blah problem", attach.Name));
        }
    }

    /*
     * do something else complex here with 
     * processingIssues and validResults
     */
}

In primo luogo, la complessità ciclomatica che mi riguarda è nata da un complesso insieme di regole aziendali che devono essere applicate che insieme riassumono un paio di insiemi di dati condivisi (qui rappresentati come processingIssues e transformedAttachments ). Fondamentalmente ciò che questo significa per me è che non solo devo rompere il foreach out nel suo metodo, ma dovrei anche considerare di rompere l'albero logico contenuto in esso in metodi aggiuntivi nell'interesse di mantenere i miei metodi "brevi". In altre parole, sembrerebbe che non sia abbastanza solo per rompere il mio metodo attuale nelle sue due parti esterne più ovvie: "pre-elaborazione" e "post-elaborazione" degli allegati, se vuoi. Quanto lontano devo andare con questo dato che mi sto occupando di un albero logico complicato con preoccupazioni che non sono così "separate" l'una dall'altra. In altre parole, se "la separazione delle preoccupazioni" è una ragione per il refactoring di metodi più piccoli, ma lo è anche per la "complessità ciclomatica" - che sembrano entrambi concetti ortogonali - che è la ragione "giusta" per il refactoring, e come dovrei farlo nel secondo caso?

In secondo luogo, al fine di separare alcune delle preoccupazioni per separare i metodi, quei metodi separati dovranno restituire più variabili (di nuovo, processingIssues e transformedAttachments ), entrambi sono solo necessari all'interno dei metodi separati stessi , quindi mi trovo di fronte alla scelta: utilizzare i parametri di output o creare una piccola classe per racchiudere i valori restituiti. Quale è meglio?

Qualche consiglio su questo argomento e sui miei specifici hang-up sarebbe molto apprezzato.

    
posta rory.ap 14.05.2015 - 19:52
fonte

2 risposte

6

Il tuo scenario sembra aver collocato un intero "sottosistema di elaborazione dei messaggi" all'interno di una singola funzione. Per semplificare la funzione, è necessario creare un sottosistema di elaborazione dei messaggi che consta di diverse classi. Alcune, se non tutte, di queste classi dovranno essere istanziate, permesse di essere eseguite e scartate su ogni invocazione del tuo metodo.

Questo sarà un sottosistema che avrà uno stato e alcune classi che implementano operatori (le regole effettive). Il problema di restituire più risultati viene trasformato nel compito di modificare più di un elemento di stato del sottosistema e il problema delle varie regole che interagiscono tra loro si trasforma anche in avere alcune regole che modificano lo stato del sottosistema mentre altre regole guardano a questo stato modificato per capire cosa fare.

Essenzialmente, ci sarebbe una stretta correlazione tra l'insieme di variabili locali della funzione e l'insieme di variabili membro della classe del sottosistema di elaborazione dei messaggi, sebbene il sottosistema potrebbe dover contenere più variabili membro per acquisire informazioni che è attualmente codificato in branching condizionale (dichiarazioni if ) nel tuo metodo esistente.

Inutile dire che il numero totale di linee in tale sottosistema potrebbe facilmente superare le attuali 350 linee di un fattore 2 o 3, quindi, prima di intraprendere un simile refactoring, valuta se ne hai davvero bisogno.

    
risposta data 15.05.2015 - 00:10
fonte
0

Vorrei pubblicare l'intera funzione sulla revisione del codice.

Ma, ecco le mie opinioni sul tuo frammento di codice e domande da un punto di vista OOP:

  • static ProcessMessage dovrebbe essere message.GetTransformedAttachments() e TransformedAttachment.Process()
  • if (logic) dovrebbe essere attach.IsLogicTrue
  • TransformedAttachmentData(attach) dovrebbe essere attach.GetTransformedData()

Ecco già altri 3 metodi!

Se si creano le mini classi è necessario restituire più valori dalle funzioni e quindi spostare le funzioni su tali classi, è quindi possibile utilizzare l'ereditarietà per rimuovere le istruzioni condizionali nella logica. Ciò ridurrà la lunghezza del tuo codice (nei metodi) e la complessità di cyclelowhatsit.

il tuo metodo potrebbe finire per essere solo,

for each x in y
   x.doSomething

se x è sottoclasse abbastanza non hai istruzioni if e il grafico degli oggetti si prende cura di tutti i tuoi output!

nota: non sto dicendo che questa sia la soluzione migliore. Una classe di servizio con una tonnellata di if può a volte essere buona se corrisponde alla definizione aziendale del processo e molta ereditarietà può rendere il debug un rompicapo. Ma dovresti provarlo e vedere se ti piace.

    
risposta data 15.05.2015 - 11:44
fonte