Un modo migliore di gestire le pre condizioni e il logging

1

1. Immagina di avere il seguente:

void Foo::doFoo()
{
    if (!isConditionValid())
    {
        log("doFoo not possible because condition is not valid");
        return;
    }

    if (!isTheOtherConditionValid())
    {
        log("doFoo not possible because the other condition is not valid");
        return;
    }

    // do Foo actions
}

C'è un modo migliore per formattare il codice? Molto spesso, ho 1,2 o 3 condizioni e trovo davvero ripetitivo avere questa struttura.

2. Che cosa succede se ho il metodo doBar, che ha le stesse identiche condizioni, quindi avremmo:

void Foo::doBar()
{
    if (!isConditionValid())
    {
        log("doBar not possible because condition is not valid");
        return;
    }

    if (!isTheOtherConditionValid())
    {
        log("doBar not possible because the other condition is not valid");
        return;
    }

    // do Bar actions
}

Mi sento come se dovessi refactoring quelle condizioni, ma trovo difficile trovare il nome del metodo. Se chiami il metodo "areConditionsValid", non mi aspetto che registri nulla, solo per restituire un valore booleano. Quindi il seguente non mi sembra giusto:

bool Foo::areConditionsValid(String action)
{
    if (!isConditionValid())
    {
        log(action + " not possible because condition is not valid");
        return false;
    }

    if (!isTheOtherConditionValid())
    {
        log(action + " not possible because the other condition is not valid");
        return false;
    }

    return true;
}

Perché fa più di quello che dice.

Non posso chiamarlo checkConditionsAndLog() perché non è una buona pratica avere un "E" nel nome del metodo, è un buon modo per vedere che sta facendo più di quello che dovrebbe fare ...

Qual è il modo migliore per refactoring questo?

Lo stesso vale se c'è una sola condizione da controllare.

PS: è in C ++

    
posta dyesdyes 04.08.2015 - 10:35
fonte

4 risposte

1

C'è qualche ambiguità con il termine "precondizioni":

  • potrebbe significare "non chiamare la funzione se questi non sono validi" (cioè, qualcosa che il codice client dovrebbe gestire)
  • potrebbe significare "se questi non sono validi nella funzione, (messaggio di registro e) non calcolare nulla"
  • potrebbe significare "se questi non sono validi, esci dall'applicazione"

Quando si implementa questo, raramente si deve decidere come gestire l'errore, nel luogo in cui si verifica; al contrario, dovresti generare un'eccezione e lasciare che il codice client decida.

Con questo, il tuo codice potrebbe diventare:

void Foo::AssertPrecondition(bool condition, std::string msg)
{
    if(!condition)
        throw std::runtime_error{ std::move(msg) };
}

void Foo::doFoo()
{
    AssertPrecondition(isConditionValid(),
        "doFoo not possible because condition is not valid");
    AssertPrecondition(isTheOtherConditionValid(),
        "doFoo not possible because the other condition is not valid");

    // TODO: implement valid execution branch
}

Da questo momento in poi, il codice cliente può decidere cosa succede quando si verifica un errore.

Inoltre, di regola, se hai più di una linea ripetitiva nel codice, dovresti rifattenerli in una funzione separata.

    
risposta data 07.08.2015 - 16:03
fonte
2

Un problema che posso vedere con il tuo approccio è che le tue precondizioni return , invece di generare un'eccezione. Di solito, le condizioni preliminari generano un'eccezione quando violate. Dato che stai usando C ++, puoi usare gli asser.

Se si utilizzano eccezioni / asserzioni, è possibile eseguire facilmente un ulteriore refactoring. Invece di:

if (!isConditionValid())
{
    log("doFoo not possible because condition is not valid");
    assert(false);
}

if (!isTheOtherConditionValid())
{
    log("doFoo not possible because the other condition is not valid");
    assert(false);
}

// do Foo actions

puoi avere un metodo come:

void Contracts::require(predicate, message)
{
    if (!predicate())
    {
        log(message);
        assert(false);
    }
}

e lo chiami in questo modo:

Contracts::require(isConditionValid, "doFoo not possible because condition is not valid");
Contracts::require(isTheOtherConditionValid, "doFoo not possible because...");

// do Foo actions
    
risposta data 04.08.2015 - 10:52
fonte
0

Utilizza semplicemente un'implementazione di contratti di codice, in alcune implementazioni come quella fornita in. Net queste condizioni possono essere verificate in fase di compilazione.

link

    
risposta data 04.08.2015 - 11:01
fonte
0

Puoi anche calcolare il risultato e loggare se fallisce.

Qualcosa del genere:

bool Foo::areConditionsValid(String action)
{
    const bool result = isConditionValid() &&
                        isTheOtherConditionValid();

    if (!result)
    {
        log(action + " not possible because some conditions are not valid");
    }

    return result;
}

Trovo anche questo più leggibile:

void Foo::doBar()
{
    if (!isConditionValid())
    {
        log("doBar not possible because condition is not valid");
    }
    else if (!isTheOtherConditionValid())
    {
        log("doBar not possible because the other condition is not valid");
    }
    else
    {
        // do Bar actions
    }
}
    
risposta data 04.08.2015 - 11:52
fonte

Leggi altre domande sui tag