Qual è un buon design per un metodo che può restituire diversi risultati logicamente diversi?

5

Il titolo della domanda è probabilmente troppo astratto, quindi permettimi di fornire un esempio particolare di ciò che ho in mente:

Esiste un servizio web che incapsula un processo di modifica delle password per gli utenti di un sistema distribuito. Il webservice accetta l'accesso dell'utente, la sua vecchia password e una nuova password. Sulla base di questo input, può restituire uno dei seguenti tre risultati:

  1. Nel caso in cui l'utente non sia stato trovato o la sua vecchia password non corrisponda, verrà semplicemente restituita con HTTP 403 Proibito.
  2. Altrimenti, richiede una nuova password e si assicura che sia conforme a una politica di password (ad es. è abbastanza lunga, contiene un corretto mix di lettere e numeri, ecc.). In caso contrario, restituirà un XML che descrive il motivo per cui la password non è conforme al criterio.
  3. Altrimenti, cambierà la password e restituirà un XML contenente una data di scadenza della nuova password.

Ora mi piacerebbe progettare una classe, idealmente con un singolo metodo, per incapsulare il funzionamento con questo servizio web. Il mio primo colpo è stato questo:

public class PasswordManagementWebService
{
    public ChangePasswordResult ChangePassword(string login, string oldPassword, string newPassword)
    {
        ChangePasswordResult result;

        // send input to websevice, it's not important how; the httpResponse
        // will contain a response from webservice
        var httpResponse;
        if (HasAuthenticationFailed(httpResponse)
        {
            throw new AuthenticationException();
        }
        else if (WasPasswordSuccessfullyChanged(httpResponse))
        {
            result = new ChangePasswordSuccessfulResult(httpResponse);
        }
        else
        {
            result = new ChangePasswordUnsuccessfulResult(httpResponse);
        }

        return result;
    }
}    

public abstract class ChangePasswordResult
{
    public abstract bool WasSuccessful { get; }
}

public abstract class ChangePasswordSuccessfulResult
{
    public ChangePasswordSuccessfulResult(HttpResponse  httpResponse)
    {
        // initialize the class from the httpResponse
    }

    public override bool WasSuccessful { get { return true; } }
    public DateTime ExpirationDate { get; private set; }
}

public abstract class ChangePasswordUnsuccessfulResult
{
    public ChangePasswordUnsuccessfulResult(HttpResponse  httpResponse)
    {
        // initialize the class from the httpResponse
    }

    public override bool WasSuccessful { get { return false; } }

    public bool WasPasswordLongEnough { get; private set; }        
    public bool DoesPasswordHaveToContainNumbers { get; private set; }
    // ... etc.         
}

Come puoi vedere, ho deciso di utilizzare classi separate per i casi di restituzione n. 2 e n. 3 - Avrei potuto usare una singola classe con un booleano, ma sembra un odore, la classe non avrebbe uno scopo chiaro . Con due classi separate, un utente della mia classe PasswordManagementWebService ora deve sapere quali classi ereditano da ChangePasswordResult e per eseguire il cast su una classe corretta in base alla proprietà WasSuccessful . Mentre ora ho delle belle lezioni focalizzate sul laser, ho reso la vita dei miei utenti più difficile di quanto dovrebbe essere.

Per quanto riguarda il caso n. 1, ho appena deciso di lanciare un'eccezione. Avrei potuto creare anche un'eccezione separata per il caso numero 2 e restituire solo qualcosa dal metodo quando la password è stata modificata con successo. Tuttavia, questo non mi sembra giusto - non credo che una nuova password non valida sia uno stato abbastanza eccezionale da giustificare il lancio di un'eccezione.

Non sono molto sicuro di come progetterei le cose se ci fossero più di due tipi di risultati non eccezionali dal webservice. Probabilmente cambierei un tipo di proprietà WasSuccessful da booleano a enum e lo rinominerei in ResultType , aggiungendo una classe dedicata ereditata da ChangePasswordResult per ogni possibile ResultType .

Infine, alla domanda attuale: questo approccio alla progettazione (ovvero che ha una classe astratta e obbliga i client a trasmettere a un risultato corretto basato su una proprietà) è corretto quando si affrontano problemi come questo ? Se sì, c'è un modo per migliorarlo (magari con una strategia diversa per quando generare eccezioni rispetto ai risultati di ritorno)? Se no, cosa consiglieresti?

    
posta Nikola Anusev 20.07.2014 - 18:06
fonte

3 risposte

5

Non abbonato alla scuola di pensiero che dice "le eccezioni dovrebbero essere solo per casi eccezionali!" Le persone hanno paura delle eccezioni per qualche motivo. Se non riesci a fare ciò che hai detto che stai per fare, lancia un'eccezione, anche se si tratta di un errore comune o previsto.

Mi piace per alcuni motivi. È impossibile che il chiamante ignori (qualcuno potrebbe facilmente dimenticare di ispezionare il valore di ritorno di un metodo che indica un errore con un codice di ritorno). È semplice (nessun valore di segnaposto speciale per risultati mancanti o gerarchie che richiedono downcasting). È atomico (o il mio cambiamento è andato a buon fine o mi hai detto di perdermi, non ho dubbi su quale sia.) È granulare (puoi allegare tutte le informazioni che vuoi all'eccezione e lanciare diverse eccezioni per diversi fallimenti).

Quindi progetterei il tuo metodo in questo modo:

public void ChangePassword(string password)
{
    HttpResponse response = RequestPasswordChange();
    if (AuthFailed(response))
        throw new AuthorizationException();
    if (PasswordWasNotChanged(response))
        throw new InvalidNewPasswordException(
            WasPasswordLongEnough(response),
            DidPasswordContainNumbers(response)
        );
}

"Riuscire-o-gettare" è una buona regola empirica, e le persone sono abituate ad esso, indipendentemente da quanto ondeggiano le loro braccia. Dopo tutto, Dictionary<TKey, TValue> genera KeyNotFoundException quando non riesci a indicizzarti. Un uso non eccezionale delle eccezioni.

    
risposta data 21.07.2014 - 01:41
fonte
3

Penso che questo approccio stia usando troppo l'ereditarietà aggiungendo ulteriore (inutile) complessità. Favorisci la composizione sull'eredità, quando possibile. Soprattutto se si tratta di una cosa Success / Non-Success che è facilmente rappresentabile con un booleano non garantisce la necessità di una sottoclasse.

È possibile includere tutto ciò che è necessario in una singola classe e, in base al risultato, è possibile utilizzare le proprietà di conseguenza. La classe ha ancora un unico scopo, per restituire informazioni sulla modifica della password. Il fatto che ci siano diversi risultati è una complessità essenziale del compito.

class ChangePasswordResult
{
    public bool Result { get; private set; }
    public DateTime ExpirationDate { get; private set; }  
    public bool WasPasswordLongEnough { get; private set; }        
    public bool DoesPasswordHaveToContainNumbers { get; private set; }
}

Modifica:

jhewlett Richiama molto bene anche la serializzazione. Con una struttura a singolo risultato restituita si ha un'interfaccia più stabile con cui i client possono interagire. Quando documenti la tua interfaccia puoi dire "questo endpoint restituisce X" invece di "questo endpoint restituisce X o Y a seconda di ..."

    
risposta data 21.07.2014 - 00:55
fonte
0

Suppongo che ci sia un'azione diversa quando WasSuccessful è true e diversa azione quando WasSuccessful è false . Perché non inserire questo codice all'interno di ChangePasswordSuccessfulResult e ChangePasswordUnsuccessfulResult e chiamarlo tramite il metodo virtuale, decidendo in tal modo in base al tipo. Quindi, non devi preoccuparti del casting.

public abstract class ChangePasswordResult
{
    public abstract SomeResponseData GetResponseData();
}

public class ChangePasswordSuccessfulResult
{
    public ChangePasswordSuccessfulResult(HttpResponse  httpResponse)
    {
        // initialize the class from the httpResponse
    }

    public DateTime ExpirationDate { get; private set; }

    public override SomeResponseData GetResponseData()
    {
        // return data for successful result
    }
}

public abstract class ChangePasswordUnsuccessfulResult
{
    public ChangePasswordUnsuccessfulResult(HttpResponse  httpResponse)
    {
        // initialize the class from the httpResponse
    }

    public bool WasPasswordLongEnough { get; private set; }        
    public bool DoesPasswordHaveToContainNumbers { get; private set; }
    // ... etc.         

    public override SomeResponseData GetResponseData()
    {
        // return data for unsuccessful result
    }
}
    
risposta data 21.07.2014 - 08:19
fonte

Leggi altre domande sui tag