Eliminare le istruzioni long / Complex if..else usando Chain of Responsibility?

3

Ho un HttpHandler, che consente agli utenti di accedere a un sistema passando un codice crittografato.

All'interno di ProcessRequest esegue alcuni passaggi.

  1. Recupera il codice crittografato dalla richiesta (potrebbe essere in Form / X-Header / Query)
  2. Decifralo. (Risultati in una stringa JSON)
  3. Deserializza in un tipo dinamico
  4. Determina il tipo di richiesta, potrebbe essere login / register / activate
  5. Convalida richiesta
  6. Carica l'abbonamento utente
  7. Carica entità utente
  8. Se Req.Type = Accedi - > Accedi utilizzando FormsAuth
  9. Se Req.Type = Registra - > Crea utente
  10. Se Req.Type = Registra - > Invia email di attivazione

Questo va avanti. Attualmente questo è qualcosa di simile a una dichiarazione long if / else che sto cercando di ridefinire. Sto pensando di utilizzare il modello Chain Of Responsibility per convertire questi passaggi in if / else in una catena che viene eseguita in sequenza. Tuttavia, questo sarà diverso dal CoR originale poiché solo gli ultimi pochi passi "manterranno" effettivamente la richiesta, i primi passi faranno solo un po 'di pre-elaborazione e faranno tappa per gli ultimi passi per eseguire il suo lavoro.

Il mio problema principale è che diversi passaggi della catena funzioneranno su diversi dati di input. I primi passi (decrittazione, deserializzazione, ecc.) Funzioneranno su stringhe mentre la seconda metà dovrebbe idealmente funzionare su oggetti dinamici deserializzati. Non mi piace come suona.

Sto cercando di adattare un coperchio rotondo a una bottiglia quadrata? Non è questo uno scenario in cui il CdR può / deve essere applicato? Ci sono altri schemi / strategie che potrei usare per risolvere questi scenari.

Ho pensato di utilizzare anche il pattern decorator, ma non mi è piaciuto molto, dato che mi piacerebbe essere in grado di attivare e disattivare determinati passaggi (es: attivazione e-mail) per alcuni scenari.

NOTA: ho visto questa domanda as bene, ma non sono sicuro che risponda alla mia domanda.

    
posta BuddhiP 28.08.2013 - 07:14
fonte

4 risposte

5

Il tuo metodo ProcessRequest sta facendo troppo in un blocco di codice. Catena di responsabilità è uno schema inappropriato perché alcuni di questi passaggi sono obbligatori e alcuni sono alternativi.

Guarda gli ultimi 3 passaggi - da 8 a 10. Il tuo metodo principale non dovrebbe preoccuparsi di ciò che deve essere fatto per ogni singolo tipo di richiesta. Tutto ciò che dovrebbe accadere qui è

  1. Il tipo di richiesta è nel mio insieme di tipi consentiti?
  2. Se sì, invoca la classe corrispondente
  3. Se no, gestisci l'errore.

Questo sarà un blocco di codice molto breve. Richiede, da qualche altra parte nel codice, di creare la classe specifica del tipo di richiesta (e le sottoclassi) e il set per contenerle, ma il vantaggio è che questo blocco di codice nel metodo principale non deve mai cambiare.

Il passaggio 5 dovrebbe essere un metodo della classe specifica del tipo.

I passaggi 6 e 7 possono probabilmente essere incapsulati in un oggetto Utente, ma significano anche solo qualcosa per gli utenti esistenti, quindi la roba relativa alla gestione degli oggetti utente dovrebbe andare all'interno della classe specifica del tipo.

Quindi abbiamo già ridotto il codice a

  1. Recupera il codice crittografato dalla richiesta (potrebbe essere in Form / X-Header / Query)
  2. Decifralo. (Risultati in una stringa JSON)
  3. Deserializza in un tipo dinamico
  4. Determina il tipo di richiesta, potrebbe essere login / register / activate
  5. se digiti set-of-allowed-types then request-type = new type-specific-class else fail.
  6. se non request-type.validate () fallisce
  7. request-type.dostuff ()

Vedi? Nessun modello coinvolto. Le catene If-then-else sono un cattivo odore, sì, ma di solito possono essere sostituite con una serie di opzioni valide o un'istruzione switch. Prova questo approccio semplice prima di andare in giro.

    
risposta data 28.08.2013 - 11:09
fonte
1

Non dimenticare di considerare le diverse responsabilità qui.
Hai

  1. un oggetto di richiesta crittografato che deve essere decrittografato
  2. un deserializzatore JSON
  3. qualcosa che determina il tipo di richiesta
  4. un validatore di richieste
  5. alcuni dati di accesso ai dati
  6. e qualcosa che sceglie il passo successivo da intraprendere

    Non ti consiglierei di mettere tutte quelle cose in una IHttpHandler (violazione SRP). Dividi se disattiva le linee di responsabilità e usa la composizione nel tuo gestore. Questo probabilmente renderà più semplice la lettura e potrai testare i diversi componenti in modo isolato.
risposta data 28.08.2013 - 08:44
fonte
1

In Chain of responsibility, un oggetto di elaborazione che non è in grado di elaborare una determinata voce lo passerà al successivo oggetto di elaborazione.

Ad esempio, se stai costruendo un sistema che elaborerà i messaggi da un dispositivo hardware, un oggetto di elaborazione (PO₁) potrebbe essere in grado di gestire i messaggi di controllo, un altro (PO₂) - errori e l'ultimo ( PO₃) - flusso di dati.

Se un messaggio non è né un messaggio di controllo, né un errore, viene passato dal PO₁ al PO₂ e dal PO₂ al PO₃ che elaborerà il messaggio o genererà un'eccezione, perché il messaggio non viene riconosciuto.

Il pattern Chain of responsibility è comodo quando devi elaborare qualcosa di uniforme, senza modificarlo.

Se si utilizza il modello Chain of responsibility, significa che:

  1. Ogni oggetto di elaborazione sarà necessario per gestire o passare un gruppo di oggetti (probabilmente raggruppati in un enorme oggetto) con informazioni su sessioni, richieste, ecc. Quando si devono passare così tante informazioni da un metodo a un altro, questo è un segno che qualcosa potrebbe essere sbagliato.

  2. Alcuni oggetti di elaborazione modificheranno il contesto. Questo non è qualcosa di attuale nel modello Chain of responsibility.

Il flusso corrente nella tua domanda sembra un normale flusso di applicazioni. Non c'è bisogno di usare Catena di responsabilità per questo. Ad esempio, in questa situazione:

  1. Verifica se il file esiste ed è leggibile.

  2. Apri il file.

  3. Leggi i suoi contenuti.

  4. Controlla l'intestazione per assicurarti che il formato sia corretto.

  5. Verifica il CRC.

  6. Contenuti di analisi.

  7. Calcola le statistiche dai contenuti analizzati.

  8. Restituisci i risultati statistici.

il flusso di lavoro è molto simile, ad esempio alcuni passaggi potrebbero indicare che altri non dovrebbero essere eseguiti: ad esempio se non è possibile analizzare i contenuti, non avrebbe senso calcolare le statistiche, o se l'intestazione è non valido, non è necessario verificare il CRC. Tuttavia, il pattern Chain of responsibility non si adatta bene qui, perché diversi passaggi utilizzano dati diversi e possono modificarlo (o creare dati aggiuntivi).

    
risposta data 28.08.2013 - 08:37
fonte
1

La domanda a cui ti sei collegato fornisce una risposta che probabilmente si applicherà anche alla tua situazione: l'introduzione del CdR rischia di rendere le cose complicate ancora più complicate. "Pattern CoR" va bene se si desidera modificare la sequenza della catena in fase di esecuzione, il che non è il caso nella tua situazione precedente.

Quindi il mio consiglio qui è:

  • ha un metodo per ciascuno dei primi sette passaggi (con un valore di ritorno se il passaggio non riesce).
  • raggruppa le chiamate di tali metodi insieme (forse tutte le 7 chiamate o il primo gruppo raggruppa i passaggi intermedi). Può risultare in un metodo come

     StatusCode PreprocessRequest()
     {
        if(!RetrieveEncryptedCodeFromRequest())
            return StatusCode.RetrieveEncryptedFailed;
    
        if(!Decrypt())
           return StatusCode.DecryptFailed;
    
        //...
        return StatusCode.OK;
      }
    

Quindi chiama quel metodo come

  ProcessRequest processor = new ProcessRequest();
  // ...
  StatusCode state = processor.PreprocessRequest();
  if(state==StatusCode.OK)
  {
      Request req = processor.GetRequest();
      HandleRequest(req);
  }
  else
  {
       LogError(state);
  }

(Naturalmente, i dettagli potrebbero essere diversi nel tuo caso, specialmente la gestione degli errori, ma spero che tu abbia capito l'idea.)

Quindi, ecco il succo: cerca di rendere il tuo "se" meno complesso semplicemente raggruppando le cose in metodi, rendendo l'errore gestendo uniforme su ogni livello, assicurati di avere parti di un metodo tutte sullo stesso livello di astrazione (altrimenti introdurre un altro metodo intermedio). Immagino che ti aiuterà a migliorare il codice fino a un punto in cui le "istruzioni if if complesse" svaniscono.

    
risposta data 28.08.2013 - 08:30
fonte

Leggi altre domande sui tag