Chiamando più parti di codice "fail-able" in una riga

Recentemente ho attraversato questo scenario esatto! Ho iniziato creando una classe Result con alcuni metodi di supporto:

public class Result
{
    public bool Success { get; set; }
    public ValidationResult[] TransactionErrors { get; private set; }

    // Combines param Result errors into this Result instance
    public Result MergeWithErrorsFrom(Result transaction)
    {
        if (transaction == null) return this;
        TransactionErrors = TransactionErrors.Concat(transaction.TransactionErrors).ToArray();
        if (TransactionErrors.Any() || !transaction.Success)
            Success = false;
        return this;
    }

    // Method will execute each function in the params Func list until one fails, combining the error messages in the current instance. Returns 'this'.
    public Result CombineWithUntilFailure(params Func<Result>[] executionFuncList)
    {
        foreach (var func in executionFuncList)
        {
            this.MergeWithErrorsFrom(func()); // execute the Func and copy errors to current instance.
            if (this.TransactionErrors.Any() || !this.Success) // stop exec'ing if we get any kind of error.
                break;
        }
        return this;
    }

    public void AddTransactionError(string errorMessage, string[] memberNames = null)
    {
        var newError = new ValidationResult(errorMessage, memberNames ?? new[] { "" });
        TransactionErrors = TransactionErrors.Concat(new[] { newError }).ToArray();
        Success = false;
    }

    public static Result FailIf(Func<bool> func, string errorMessage)
    {
        var callResult = new Result();
        if (func())
            callResult.AddTransactionError(errorMessage);
        return callResult;
    }

    public static Result Empty() => new Result { Success = true };

    public static Result Empty(Action action)
    {
        action();
        return new Result { Success = true };
    }

    public static Result CombineUntilFailure(params Func<Result>[] executionFuncList) =>
        new Result().CombineWithUntilFailure(executionFuncList);
}

public class Result<T> : Result
{
    public T Payload { get; set; }
}

Successivamente, mi sono assicurato che tutti i miei metodi di validazione e transazionali restituissero un Result o Result<T> . In questo modo, avrei potuto concatenarli usando la funzione CombineWithUntilFailure dall'alto. Il codice risultante sarebbe simile a questo:

using (var storeContext = StoreContext.CreateContext())
{
    var releaseNeeded = false;
    var result = Result.CombineUntilFailure(
        () => storeContext.SingleUserMode(),
        () => Result.Empty(() => releaseNeeded = true),
        () => storeContext.BeginEdit(),
        () => storeContext.SaveChanges());

    if (releaseNeeded)
        result.CombineWithUntilFailure(() => storeContext.ReleaseSingleUserMode());

    if (!result.Success) { /*show errors here*/  }

    return result;
}

Se uno qualsiasi dei parametri Func sopra restituisce un risultato non valido, i cortocircuiti della catena e i seguenti sono not eseguiti. Il mio codice effettivo per la classe Result include cose come la messaggistica di successo extra per la registrazione e così via, che possono essere compilati nello stesso modo in cui i messaggi di errore sono, se hai bisogno di quella funzionalità.

Ho imparato questa tecnica da un articolo che la descriveva come "Programmazione orientata alle ferrovie" trovata qui: link

Mi manca qualcosa? non hai bisogno di nulla di intelligente qui

using (var storeContext = StoreContext.CreateContext())
{
    try
    {
        storeContext.SingleUserMode();
        storeContext.BeginEdit();
        storeContext.SaveChanges();
        //update UI on success
    }
    catch(SingleUserModeException ex)
    {
        //fire messagebox on error
        throw; //prevent further execution
    }
    catch(Exception ex)
    {
        //fire messagebox on error
        //alternatively append errors to a list for later display
    }

    //continue because the single user mode did not fail
    try
    {
        storeContext.ReleaseSingleUserMode();
    }
    catch
    {
        //fire messagebox on error
    }
}

Sono a favore di oggetti componibili, quindi pattern Decorator. Vengo con oggetti piccoli, coesivi e resuable. Sembra più simile a OOP e non dimenticherò, per esempio, di chiudere la transazione avviata e allo stesso modo. Quindi ecco come lo farei (mi dispiace per php):

interface Response
{
    public function display();
}

interface Action
{
    public function go(): Response;
}

class SingleModeOn
{
    private $a;
    private $c;

    public function __construct(Action $a, Context $context)
    {
        $this->a = $a;
        $this->c = $context;
    }

    public function go(): Response
    {
        if ($this->c->singleUserMode()) {
            $r = $this->a->go();
            $this->c->releaseSingleUserMode();
            return $r;
        } else {
            return new SingleModeFailed();
        }
    }
}

class Transactional implements Action
{
    private $a;
    private $c;

    public function __construct(Action $a, Context $context)
    {
        $this->a = $a;
        $this->c = $context;
    }

    public function go(): Response
    {
        if ($this->c->beginEdit()) {
            $response = $this->a->go();
            $this->c->finishEdit();
            return $response;
        } else {
            return new TransactionFailed();
        }
    }
}

(new SingleModeOn(
    new Transactional(
        new YourItemSavingScenarioName(
            new UINotification()
        )
    )
))
    ->go()
        ->display()
;

Nessun nidificato if s, nessuna eccezione per il controllo del flusso.

Raccomando senz'altro le eccezioni. Dopo un po 'di hacking, sono arrivato a questo.

class Program
{
    static void Main(string[] args)
    {
        using (var storeContext = StoreContext.CreateContext())
        {
            try
            {
                var releasable = storeContext.SingleUserModeAsReleasable();
                releasable.ReleaseAfter(() =>
                {
                    //load data into storecontext...

                    storeContext.BeginEdit(); // will throw exception when error

                    //change data...

                    storeContext.SaveChanges(); // will throw exception when error

                    //update UI on success
                });
            }
            catch (Exception)
            {
                //fire messagebox on error
            }
        }
    }
}

public interface IReleasable
{
    void Release();
}

public static class ReleasableExtension
{
    public static void ReleaseAfter(this IReleasable releasable, Action action)
    {
        Exception[] exceptions = new Exception[2];
        try
        {
            action();
        }
        catch (Exception ex)
        {
            exceptions[0] = ex;
        }

        try
        {
            releasable.Release();
        }
        catch (Exception ex)
        {
            exceptions[1] = ex;
        }

        var thrownExceptions = exceptions.Where(x => x != null).ToArray();
        if(thrownExceptions.Length != 0)
        {
            throw new AggregateException(thrownExceptions);
        }
    }
}

public static class StoreContextSingleUserMode
{
    private class SingleUserModeRelease : IReleasable
    {
        private StoreContext storeContext;

        public SingleUserModeRelease(StoreContext storeContext)
        {
            this.storeContext = storeContext;
        }

        public void Release()
        {
            storeContext.ReleaseSingleUserMode(); // will throw exception when error
        }
    }

    public static IReleasable SingleUserModeAsReleasable(this StoreContext storeContext)
    {
        storeContext.SingleUserMode(); // will throw exception if error, won't even bother releasing

        return new SingleUserModeRelease(storeContext);
    }
}

Il problema principale è la necessità di rilasciare SingleUserMode se è stato acquisito con successo. Ecco perché l'ho incapsulato insieme in un oggetto IReleasable . Ciò consente quindi di creare codice simile alla continuazione che gestisca correttamente lo stato eccezionale. Un problema estetico è che il codice è dentro e fuori. Quindi non è ovvio come fluirà.

Questo è fondamentalmente il clone dell'interfaccia IDisposable . Ma IDisposable non può essere utilizzato, poiché non deve generare un'eccezione da Dispose . E anche allora, se c'è un'eccezione nel blocco finally, quindi precedente l'eccezione è persa . Quale non è il comportamento che vuoi. Così ho creato un clone con un comportamento corretto, che genera un'eccezione aggregata se sia il codice interno che il codice di rilascio generano un'eccezione.

Che cosa significa "bello"? Se intendi "più facile da leggere e gestire", dovresti utilizzare un codice C # più idiomatico. Il modo idiomatico di segnalare gli errori in C # avviene attraverso le eccezioni. L'uso di un flag di valore risultato per segnalare errori non è unidirezionale, il che significa che non è piacevole per il lettore.

Se modifichi il codice per utilizzare le eccezioni, potrebbe apparire come segue:

using (var storeContext = StoreContext.CreateContext())
{
    try 
    {
        //load data into storecontext...
        storeContext.BeginEdit();
        //change data...
        storeContext.SaveChanges();
        //update UI on success
    }
    catch (Exception ex)
    {
      //fire messagebox on error
    }
} 

Ho omesso SingleUserMode e ReleaseSingleUserMode poiché dichiari che non sono necessari. Non dovresti avere un codice che non ha uno scopo. Ma se si scopre che hai bisogno di questa coppia di metodi, ti suggerisco di incapsularli in IDsposable class, poiché in questo modo puoi garantire che la modalità utente singolo sia sempre rilasciata, ad es. come:

using (new SingleUserMode()) 
{
  // do something in single user mode
}

Supponendo che è necessario implementarlo più volte, è possibile creare qualcosa come una classe di transazione, come di seguito:

public void Execute() {
    using (var storeContext = StoreContext.CreateContext())
    {
        var caller = new Caller<StoreContext>(storeContext);

        caller.RegisterSetup(s => s.SingleUserMode(), s => s.ReleaseSingleUserMode());
        caller.OnSuccess(s => UpdateUi());
        caller.OnError(s => ShowMessageBoxWithErrorMessages());

        //Your transaction
        caller.Call(s => {
            //load data into storecontext...
            s.BeginEdit();
        });

        caller.Call(s => {
            //change data...
            s.SaveChanges();
        });

        //This will call all Setup functions; and keep calling all functions from "Call" calls; for errors it will call "OnError", otherwise "OnSuccess" at the end, and finally the "Teardown" functions registered along with the "Setup" functions.
        caller.Commit();
    }
}

Il tuo fattore limitante è che hai un risultato di ritorno che ti serve per gestire gli aggiornamenti dell'interfaccia utente. Ciò elimina la possibilità di creare un nuovo IDisposable per gestire lo stato SingleUserMode . Supponiamo che tu abbia un nuovo oggetto per rappresentare la modalità:

public class SingleUserMode : IDisposable
{
    final StoreContext context;

    public SingleUserMode(StoreContext contextIn)
    {
        context = contextIn;
        IsValid = context.SingleUserMode();
    }

    public bool IsValid { get; }

    public void Dispose()
    {
        context.ReleaseSingleUserMode();
    }
}

Ciò consente al StoreContext di disporre di un nuovo metodo per utilizzare questo oggetto monouso:

public SingleUserMode EnterSingleUserMode()
{
    return new SingleUserMode(this);
}

Che a sua volta consente di semplificare un po 'il codice:

using (var storeContext = StoreContext.CreateContext())
using (var singleUser = storeContext.EnterSingleUserMode())
{
    if (singleUser.IsValid)
    {
        // do good stuff
    }
    else
    {
        //fire messagebox on error
    }
}

Per trarre pienamente vantaggio da questo schema, dovrai rifattorizzare alcune cose, ma si spera non male. Il costrutto using funziona con ogni IDisposable e è garantito che Dispose() venga chiamato una volta che lasci l'ambito dell'istruzione using . È un ottimo idioma in C # da utilizzare per operazioni che richiedono la pulizia.

Ciò che complica le cose è il modo in cui la tua logica attuale funziona per lo stato buono / cattivo.

Questo tipo di struttura suggerisce di utilizzare la valutazione di cortocircuito delle espressioni booleane. L'esecuzione si interrompe quando un metodo restituisce false. Sposta anche l'elaborazione extra sui condizionali alle funzioni di supporto.

    result=    storeContext.SingleUserMode()
            && handle_SingleUserMode()     // this should always return true
            && storeContext.BeginEdit()
            && handle_BeginEdit()          // this should always return true
            && storeContext.SaveChanges()
            && handle_SaveChanges();       // this should always return true

Penso che sia facile leggere il codice.

Questo dovrebbe essere migliore di tutti i if ...

try
{
    SingleUserMode();
    BeginEdit();
    SaveChanges();
}
catch (Exception ex)
{
    //Log
}
finally
{
    //Cleanup
    ReleaseSingleUserMode();
}

O forse questo ...

try
{
    using (var singleUserMode = SingleUserMode())
    {
        BeginEdit();
        SaveChanges();
    } //Release happens on Dispose...
}
catch (Exception ex)
{
    //Log
}

Anche se, penso che questo sia un abuso minore di IDisposable.

Poiché la tua logica di gestione degli errori è lineare, qualsiasi struttura nidificata, siano essi try blocchi o nidificati if , non è adatta a esprimerla. Pertanto suggerisco di utilizzare l'istruzione goto come mostrato di seguito. Ho espresso le operazioni di caricamento dei dati, di modifica e aggiornamento dell'interfaccia utente, come metodi che restituiscono un contrassegno di successo e assegnano un messaggio di errore a un parametro out string :

    storeContext = StoreContext.CreateContext();
    result = storeContext.SingleUserMode();
    if( !result ) goto Error;

    if( !LoadData( storeContext, out errMsg ) )
    {   result = NewResult( errMsg );
        goto ErrorSm;
    }

    result = storeContext.BeginEdit();
    if( !result ) goto ErrorSm;

    if( !ChangeData( storeContext, out errMsg ) )
    {   result = NewResult( errMsg );
        goto ErrorSm;
    }

    result = storeContext.SaveChanges();
    if( !result ) goto ErrorSm;

    // why can't you update UI in the very end?
    if( !UpdateUiOnSuccess( out errMsg ) )
    {   result = NewResult( errMsg );
        goto ErrorSm;
    }

ErrorSm:
    releaseResult = storeContext.ReleaseSingleUserMode();
    if( !releaseResult ) result.AddSubResult( releaseResult );

Error: storeContext.Dispose()

if( !result )
{   /* show error */ }
else
{   /* I suggest to update UI here! */ }

Ma ancora non capisco quale sia il tuo oggetto result . Lo stai usando come booleano, eppure ha un metodo AddSubResult() . Tali problemi di leggibilità sono una delle mie obiezioni alla parola chiave var .

Modifica: qui c'è una versione senza goto , con% reintegratousing() e un flag booleano che determina se entrare in modalità utente singolo (che hai detto essere facoltativo):

    using( storeContext = StoreContext.CreateContext() )
    {   while( true )
        {   if( singleUser )
            {   result = storeContext.SingleUserMode();
                if( !result ) break;
                singleUserEntered = true;
            }

            if( !LoadData( storeContext, out errMsg ) )
            {   result = NewResult( errMsg );
                break;
            }

            result = storeContext.BeginEdit();
            if( !result ) break;

            if( !ChangeData( storeContext, out errMsg ) )
            {   result = NewResult( errMsg );
                break;
            }

            result = storeContext.SaveChanges();
            if( !result ) break;

            if( !UpdateUiOnSuccess( out errMsg ) )
            {   result = NewResult( errMsg );  }

            break;
        }

        if( singleUserEntered )
        {   releaseResult = storeContext.ReleaseSingleUserMode();
            if( !releaseResult ) result.AddSubResult( releaseResult );
        }
    }