Codifica delle migliori pratiche C++

Question

Mi chiedo, c'è un modo migliore per scrivere codice quando ci sono funzioni con ritorni di stato.

Di seguito è riportato un esempio. (Si prega di ignorare gli errori di codice semplici se ce ne sono. Sono in particolare parlando della struttura. Inoltre, io sono al lavoro e non hanno un compilatore su questo computer)

#include "Session.h" 

Session::Session(const char * IPaddress, unsigned int openPort) 
{ 
    ssh_session mySession; 
    hostIP = IPaddress; 
    port = openPort; 
} 

int Session::cBeginSession() 
{ 
    try 
    { 
     int status = ssh_options_set(mySession, SSH_OPTIONS_HOST, &hostIP); 
     if (status == 0) 
     { 
      status = ssh_options_set(mySession, SSH_OPTIONS_LOG_VERBOSITY,      
            SSH_LOG_PROTOCOL); 
      if(status == 0) 
      { 
       status = ssh_options_set(mySession, SSH_OPTIONS_PORT, &port); 
       if (status == 0) 
       { 
        std::cout << "Session started\n"; 
        return 0; 
       } 
       else 
       { 
        std::cout << "Unable to set port\n"; 
        return -3; 
       } 

      } 
      else 
      { 
       std::cout << "Protocol option log verbosity unable to set\n"; 
       return -2; 
      } 
     } 
     else 
     { 
      std::cout << "Unable to set Host address\n"; 
      return -1; 
     } 
    } 
    catch (...) 
    { 
     std::cout << "Unknown exception occurred\n"; 
     return -8; 
    } 
} 

Io di solito uso se- altre istruzioni con i parametri di stato, ma tendo a finire con nidi grandi di istruzioni if-else se ci sono più di una o due funzioni coinvolte. C'è un modo più leggibile per scrivere qualcosa del genere? Diventa un nido di topi molto rapidamente.

MODIFICA: Grazie per tutte le risposte. Penso di avere alcune idee su come strutturare meglio il mio codice. Apprezzo tutti i suggerimenti diligenti.

Answer 1

Nella moderna programmazione C++, in genere, se si verifica un errore in cui il programma non può continuare, quindi penso che sia meglio fare un'eccezione a throw.

Quindi la vostra funzione non restituirebbe nulla (vale a dire void). Ogni volta che si è imbattuto in una situazione di non poter continuare, si dovrebbe throw un'eccezione che indica quale sia l'errore. Il codice chiamante gestirà quindi l'errore.

Il vantaggio di questo è che è possibile scegliere dove per gestire l'errore. Ad esempio, lo stack può scaricare tutto fino a main.

codice Si potrebbe assomigliare a questo:

void Session::cBeginSession() 
{ 
    if (ssh_options_set(mySession, SSH_OPTIONS_HOST, &hostIP)) 
    { 
     // throw an exception 
    } 
    if (ssh_options_set(mySession, SSH_OPTIONS_LOG_VERBOSITY, SSH_LOG_PROTOCOL)) 
    { 
     // throw an exception 
    } 
    if (ssh_options_set(mySession, SSH_OPTIONS_PORT, &port)) 
    { 
     // throw an exception 
    } 
} 

Una volta a ottenere il blocco di codifica con le eccezioni, il codice tende ad essere più pulito e più robusto visto che non stai sempre preoccuparsi di codice di verifica di ritorno.

EDIT

Per rispondere si commento. Puoi scegliere come e quando gestire l'errore. Puoi semplicemente catturare l'eccezione sopra la tua chiamata. Ma, in generale, se vuoi fare qualcosa che può fallire (ma non terminare un programma) puoi fare un'altra funzione che restituisce uno stato booleano.

bool Session::tryCBeginSession()

Ora, la funzione originale void Session::cBeginSession() sarebbe stata attuata in termini di questa nuova funzione. Ho scoperto che nella maggior parte dei casi la scrittura di queste doppie funzioni viene eseguita solo in un numero limitato di casi.

Answer 2

mi piace per ridurre la nidificazione, in questo modo:

status = fcn1(); 
if (status == 0) 
{ 
    // something good 
    status = fcn2(); 
} 
else 
{ 
    // something bad happened. report, and leave status reporting failure. 
} 

if (status == 0) 
{ 
    // something good 
    status = fcn3(); 
} 
else 
{ 
    // something bad happened. report, and leave status reporting failure. 
} 

if (status == 0) 
{ 
    // something good 
    status = fcn4(); 
} 
else 
{ 
    // something bad happened. report, and leave status reporting failure. 
} 

mi piace che la stampa l'errore è vicino al verificarsi dell'errore. Certo, quando si verifica un errore, il status viene controllato più volte. Ma è un piccolo prezzo da pagare per la semplicità.

Ciò si presta bene anche alla disassegnazione delle risorse e alla chiusura dei file alla fine, indipendentemente da dove si verifica l'errore.

Answer 3

In questo particolare caso (manipolazione codici di errore), che sto sostenendo per i ritorni presto:

int status = ssh_options_set(mySession, SSH_OPTIONS_HOST, &hostIP); 
if (status != 0) 
{ 
    std::cout << "Unable to set Host address\n"; 
    return -1; 
} 

status = ssh_options_set(mySession, SSH_OPTIONS_LOG_VERBOSITY,      
         SSH_LOG_PROTOCOL); 
if (status != 0) 
{ 
    std::cout << "Protocol option log verbosity unable to set\n"; 
    return -2; 
} 

status = ssh_options_set(mySession, SSH_OPTIONS_PORT, &port); 
if (status != 0) 
{ 
    std::cout << "Unable to set port\n"; 
    return -3; 
} 

std::cout << "Session started\n"; 
return 0; 

trovo il codice molto più leggibile perché ha timore di nidificazione e la gestione degli errori è mantenuto vicino al punto in cui si è verificato l'errore anziché essere sepolto in un altro ramo.

Se si decide che è meglio utilizzare le eccezioni anziché i codici di errore, è possibile mantenere la stessa struttura e sostituire i ritorni con i lanci.

Answer 4

• Non è necessario if-else se si sia o returnthrow nella dichiarazione che segue (questo è un buon argomento per throw ing BTW). Plain if farà.
• Il tipo di messaggi che si sta stampando di solito si adatta meglio a stderr anziché a stdout (cerr anziché a cout).
• Se si decide si può mantenere con stati di errore, costanti simboliche (o enumerazioni o definisce) sono di solito preferito su "numeri magici"

Answer 5

Questo è uno scenario abbastanza ideale per di gestione delle eccezioni (almeno come era inteso). La gestione delle eccezioni è in genere appropriata per gestire errori di input esterni, come in questo caso in cui l'input esterno proviene da un socket.

Hai già un blocco try/catch, ma ti suggerisco di eliminarlo poiché non c'è alcun codice di ripristino. Tieni i tuoi blocchi try/catch generalmente concentrati intorno alle aree in cui effettui una transazione di modifica. Catturare un'eccezione quindi ripristina le modifiche, riporta il sistema a uno stato valido e, in alcuni casi, emette alcuni messaggi.

Qualcosa di simile a questo:

void Session::cBeginSession() 
{ 
    if (ssh_options_set(mySession, SSH_OPTIONS_HOST, &hostIP) != 0) 
     throw runtime_error("Unable to set Host address"); 

    if (ssh_options_set(mySession, SSH_OPTIONS_LOG_VERBOSITY, SSH_LOG_PROTOCOL) != 0) 
     throw runtime_error("Protocol option log verbosity unable to set."); 

    if (ssh_options_set(mySession, SSH_OPTIONS_PORT, &port) != 0) 
     throw runtime_error("Unable to set port"); 
    std::cout << "Session started\n"; 
} 

Vediamo il codice del client di chiamare questa funzione cattura l'eccezione in un sito dove è appropriato per gestire e recuperare dall'errore. Basta preoccuparsi di lanciare l'eccezione in modo appropriato nel caso di questi errori di input esterni.

Si noti che la gestione delle eccezioni è in genere ultra economica nei casi non eccezionali (dove non si effettuano) con ottimizzazioni come EH a costo zero. Tuttavia, questi tipi di ottimizzazioni del compilatore di gestione delle eccezioni rendono il caso raro molto più lento in cui effettivamente si genera un'eccezione. Quindi le eccezioni dovrebbero essere utilizzate per casi veramente eccezionali derivanti da un qualche tipo di input esterno che il software normalmente non può gestire, come in questo caso.

Un'altra avvertenza rilevante in alcuni tipi di sistemi più grandi (architetture di plug-in, ad esempio), è che in genere le eccezioni non devono essere gettate oltre i limiti del modulo.

Questo è un po 'supponente ma non consiglio di avere molti rami di cattura basati sul tipo di eccezione (come comunemente si trova in Java, ad es.). Spesso non è necessario distinguere il tipo effettivo di un'eccezione tanto quanto inoltrare un messaggio all'utente, ad es. Cattura le eccezioni nel modo più generale/grossolano possibile e mantieni i blocchi try/catch al minimo (mentalità orientata alle transazioni di alto livello: le transazioni hanno esito positivo o falliscono e si ripristinano nel suo complesso).

Altrimenti le eccezioni possono davvero semplificare questo tipo di casi e un'intera parte della libreria C++ (e anche parti della lingua) generano eccezioni normalmente (penso davvero che C++ e gestione delle eccezioni siano inseparabili), quindi può essere utile per farne uso poiché in genere un programma robusto dovrà generalmente catturarli comunque.

Answer 6

alcuni suggerimenti annotati nel codice:

// improvement - derive your own descriptive exception FROM A STANDARD EXCEPTION TYPE 
struct session_error : std::runtime_error 
{ 
    using std::runtime_error::runtime_error; 
}; 

// improvement in constructor: initialiser lists 
Session::Session(const char * IPaddress, unsigned int openPort) 
: mySession() 
, hostIP(IPaddress) 
, port(openPort) 
{ 
} 

namespace { 
    // use of anonymous namespace so this wrapper function does not pollute other compilation units 
    void setopt(ssh_session& session, int opt, const void* val, const char* context) 
    { 
     if (ssh_options_set(session, opt, val)) 
      throw session_error(context); 
    } 

} 

void Session::cBeginSession() 
{ 
    // improvement - defer to wrapper function that handles nasty return code logic 
    // and throws a sensible exception. Now your function is readable and succinct. 

    setopt(mySession, SSH_OPTIONS_HOST, &hostIP, "setting host option"); 
    setopt(mySession, SSH_OPTIONS_LOG_VERBOSITY, SSH_LOG_PROTOCOL, "setting verbosity"); 
    setopt(mySession, SSH_OPTIONS_PORT, &port, "can't set port"); 

    std::cout << "Session started\n"; 
}