2010-11-10 11 views
7

C'è un modo migliore per scrivere questo codice senza utilizzare goto? Sembra imbarazzante, ma non riesco a pensare a un modo migliore. Devo essere in grado di eseguire un tentativo, ma non voglio duplicare alcun codice.Un modo migliore per scrivere la logica riprova senza goto

public void Write(string body) 
{ 
    bool retry = false; 
RetryPoint: 
    try 
    { 
     m_Outputfile.Write(body); 
     m_Outputfile.Flush(); 
    } 
    catch (Exception) 
    { 
     if(retry) 
      throw; 
     // try to re-open the file... 
     m_Outputfile = new StreamWriter(m_Filepath, true); 
     retry = true; 
     goto RetryPoint; 
    } 
} 
+10

dispiace, non può resistere! http://xkcd.com/292/ – Joe

+2

C'è SEMPRE un modo migliore per scrivere la logica senza un goto. –

+1

@ McWafflestix: non sono d'accordo. Ci sono alcuni * casi molto rari * in cui l'utilizzo di goto' produce in effetti un codice più pulito: l'interruzione di cicli annidati è un esempio comunemente citato (poiché C# non ha interruzioni etichettate come Java). Vedi http://stackoverflow.com/questions/2542289/is-there-ever-a-reason-to-use-goto-in-modern-net-code per ulteriori informazioni. – Heinzi

risposta

15

Ecco la logica di base che avrei usato al posto di un goto:

bool succeeded = false; 
int tries = 2; 

do 
{ 
    try 
    { 
     m_Outputfile = new StreamWriter(m_Filepath, true); 
     m_Outputfile.Write(body); 
     m_Outputfile.Flush(); 
     succeeded = true; 
    } 
    catch(Exception) 
    { 
     tries--; 
    } 
} 
while (!succeeded && tries > 0); 

Ho appena # aggiunto della logica tentativi, anche se la domanda iniziale non ha avuto alcun.

+0

Esegue tentativi infiniti, utilizza la sintassi C++ per il rilevamento e non retrocede. Non ho idea del motivo per cui è stato upvoted quando è chiaramente sbagliato. –

+0

Oh, e non si riapre in caso di fallimento. –

+2

@Steven Sudit: il corpo del blocco di cattura è implicito per essere lo stesso del codice originale. – LBushkin

1

Cosa succede se lo metti in loop? Qualcosa di simile a questo, forse.

while(tryToOpenFile) 
{ 
    try 
    { 
     //some code 
    } 
    catch 
    { 
    } 
    finally 
    { 
     //set tryToOpenFile to false when you need to break 
    } 
} 
+0

Questo ha tentativi infiniti e generalmente non risolve il problema. –

+0

Sì, ecco perché ho detto * qualcosa di simile *. L'idea era di farlo in un ciclo, che sembra essere l'idea generale in ogni risposta qui. – Joe

+0

Bene, usare un loop è abbastanza ovvio (anche se non universale). Il diavolo è nei dettagli. –

0

provare qualcosa di simile al seguente: Soluzione

int tryCount = 0; 
bool succeeded = false; 

while(!succeeded && tryCount<2){ 
    tryCount++; 
    try{ 
     //interesting stuff here that may fail. 

     succeeded=true; 
    } catch { 
    } 
} 
+0

Il bool non è necessario. Più precisamente, questo non getta l'ultimo fallimento. –

1
public void Write(string body, bool retryOnError) 
{ 
    try 
    { 
     m_Outputfile.Write(body); 
     m_Outputfile.Flush(); 
    } 
    catch (Exception) 
    { 
     if(!retryOnError) 
      throw; 
     // try to re-open the file... 
     m_Outputfile = new StreamWriter(m_Filepath, true); 
     Write(body, false); 
    } 
} 
+0

Ricorsione? Veramente? –

+0

@Steven: Certo, perché no? È una semplice ricorsione di coda e molto leggibile: 'Write (body, false)' documenta in modo abbastanza chiaro l'intenzione dello scrittore (riprova lo stesso ma non riprovare) ed evita di ingombrare il codice (nessun ciclo, nessuna variabile "riuscita"). Sostituire 'retryOnError' con un numero di tentativi è anche un esercizio facile ... – Heinzi

+0

Alcuni motivi. A causa del modo in cui GC funziona, la ricorsività mantiene gli oggetti in vita più a lungo, quindi dovrebbe essere evitato quando una soluzione iterativa è sufficientemente chiara. Quando l'ottimizzazione della ricorsione di coda è possibile, questo è meno di un problema. Questo esempio non ricorre più di una volta, ma cambieresti il ​​bool in un conto alla rovescia per tentativi se volessi aumentarlo (e probabilmente lo faresti). Un tale cambiamento amplificerebbe gli effetti. –

5

di Michael non abbastanza soddisfano i requisiti, che sono per ritentare un numero fisso di volte, gettando l'ultimo fallimento.

Per questo, vorrei raccomandare un ciclo semplice per il conto alla rovescia. Se ci riesci, esci con pausa (o, se conveniente, ritorna). Altrimenti, lasciare il controllo catch per vedere se l'indice è inferiore a 0. In tal caso, rilancia invece di registrare o ignorare.

public void Write(string body, bool retryOnError) 
{ 
    for (int tries = MaxRetries; tries >= 0; tries--) 
    { 
     try 
     { 
      _outputfile.Write(body); 
      _outputfile.Flush(); 
      break; 
     } 
     catch (Exception) 
     { 
      if (tries == 0) 
       throw; 

      _outputfile.Close(); 
      _outputfile = new StreamWriter(_filepath, true); 
     } 
    } 
} 

Nell'esempio sopra, un reso sarebbe andato bene, ma volevo mostrare il caso generale.

+1

'if (try == 0)' –

+0

@Anthony: Grazie, è stato un errore. Lo aggiusterò immediatamente. –

+0

Ho appena fatto un altro cambiamento che penso sia applicabile a qualsiasi soluzione: l'ho reso 'Close' il vecchio streamwriter prima di aprirne uno nuovo. Se questo non viene fatto, allora il vecchio manterrebbe aperto un handle fino a quando GC non entrerà in azione, bloccando il funzionamento di nuovi! –

4

@Michael's answer (con un blocco catch correttamente implementato) è probabilmente il più semplice da utilizzare nel tuo caso ed è il più semplice. Ma nell'interesse di presentare alternative, qui è una versione che i fattori di controllo di flusso "riprovare" in un metodo separato:

// define a flow control method that performs an action, with an optional retry 
public static void WithRetry(Action action, Action recovery) 
{ 
    try { 
     action(); 
    } 
    catch (Exception) { 
     recovery(); 
     action(); 
    } 
} 

public void Send(string body) 
{ 
    WithRetry(() => 
    // action logic: 
    { 
     m_Outputfile.Write(body); 
     m_Outputfile.Flush(); 
    }, 
    // retry logic: 
    () => 
    { 
     m_Outputfile = new StreamWriter(m_Filepath, true); 
    }); 
} 

Si potrebbe, naturalmente, migliorare questo con le cose come i conteggi tentativi, una migliore propagazione degli errori, e così via.

+0

Non sono davvero sicuro di cosa fare di questo. Probabilmente, questo tipo di soluzione guidata dai delegati è elegante, flessibile e riutilizzabile. Poi di nuovo, attualmente non fa realmente ciò che è necessario. Alla fine, non voglio né alzare né abbassare il voto. –

+0

@Steven: mi sono perso qualcosa? In che modo non soddisfa i requisiti? – LBushkin

+0

I modi che hai menzionato: non hanno un conteggio dei tentativi, non lanciano l'ultimo tentativo, ecc. Senza dubbio puoi * fare * fare ciò che è necessario - Ho visto abbastanza delle tue risposte per essere certo di quello - ma non l'hai fatto in questo momento. –

0

con un valore booleano

public void Write(string body) 
{ 
     bool NotFailedOnce = true;    
     while (true) 
     { 
      try 
      { 
       _outputfile.Write(body); 
       _outputfile.Flush();   
       return;      
      } 
      catch (Exception) 
      { 
       NotFailedOnce = !NotFailedOnce; 
       if (NotFailedOnce) 
       { 
        throw; 
       } 
       else 
       { 
        m_Outputfile = new StreamWriter(m_Filepath, true); 
       } 
      } 
     }   
} 
+0

Questo non può funzionare. Per prima cosa, 'NotFailedOnce =! NotFailedOnce)' sarà sempre falso, quindi non verrà mai lanciato. Invece, si collegherà per sempre. –

+0

@Steve, guarda di nuovo il codice, non è un controllo di uguaglianza, ma un compito. – Jimmy

+0

è intelligente, significa cattivo. Il compilatore genererà avvertimenti per questo, e dovrebbe, dal momento che un assegnamento nel mezzo di un predicato è molto probabilmente dovuto a un refuso. Allo stesso modo, è intelligente/cattivo impostare un bool su falso annotandolo anziché, sai, impostandolo su false. Questo codice è inutilmente offuscato. –

Problemi correlati