Si tratta di un chiaro uso di goto?

Question

Basta chiedersi se questo è considerato un chiaro uso del goto in C#:

IDatabase database = null; 

LoadDatabase: 
try 
{ 
    database = databaseLoader.LoadDatabase(); 
} 
catch(DatabaseLoaderException e) 
{ 
    var connector = _userInteractor.GetDatabaseConnector(); 
    if(connector == null) 
     throw new ConfigException("Could not load the database specified in your config file."); 
    databaseLoader = DatabaseLoaderFacade.GetDatabaseLoader(connector); 
    goto LoadDatabase; 
} 

mi sento come questo è ok, perché il frammento è piccolo e deve avere un senso. C'è un altro modo in cui le persone di solito si riprendono da errori come questo quando si desidera riprovare l'operazione dopo aver gestito l'eccezione?

Modifica: Era veloce. Per rispondere ad alcune domande e chiarire un po 'le cose, questo fa parte di un processo che si sta essenzialmente convertendo da un diverso tipo di progetto. La chiamata _userInteractor.GetDatabaseConnector() è la parte che determinerà se l'utente desidera riprovare (possibilmente con un database diverso da quello nella configurazione da cui sta caricando). Se restituisce null, non è stata specificata alcuna nuova connessione al database e l'operazione dovrebbe fallire completamente.

Non ho idea del motivo per cui non ho pensato di utilizzare un ciclo while. Deve essere troppo vicino alle 17:00.

Modifica 2: Ho dato un'occhiata al metodo LoadDatabase() e genererà un DatabaseLoaderException se fallisce. Ho aggiornato il codice sopra per catturare quell'eccezione invece di Eccezione.

Edit 3: Il consenso generale sembra essere che

• Utilizzando goto qui non è necessario - un ciclo while farà bene.
• L'utilizzo di eccezioni come questa non è una buona idea - non sono sicuro di cosa sostituirlo con.

Answer 1

C'è un altro modo la gente di solito recuperare da errori come questo, quando si vuole ripetere l'operazione dopo gestire l'eccezione?

Sì, nel codice di chiamata. Lascia che chi chiama questo metodo decida se è necessario riprovare la logica oppure no.

UPDATE:

Per chiarire, si dovrebbe prendere solo eccezioni se si può effettivamente gestire. Il tuo codice in pratica dice:

"Non ho idea di cosa sia successo, ma tutto quello che ho causato tutto per blow up ... così lascia farlo di nuovo."

Cattura errori specifici che è possibile recuperare e lasciare che il resto bolla al livello successivo da gestire. Eventuali eccezioni che raggiungono la cima rappresentano veri bug a quel punto.

UPDATE 2:

Ok, quindi piuttosto che continuare una discussione piuttosto lunga via i commenti che mi dilungherò con un esempio di codice semi-pseudo.

L'idea generale è che è sufficiente ristrutturare il codice per eseguire test e gestire meglio l'esperienza utente.

//The main thread might look something like this 

try{ 
    var database = LoadDatabaseFromUserInput(); 

    //Do other stuff with database 
} 
catch(Exception ex){ 
    //Since this is probably the highest layer, 
    // then we have no clue what just happened 
    Logger.Critical(ex); 
    DisplayTheIHaveNoIdeaWhatJustHappenedAndAmGoingToCrashNowMessageToTheUser(ex); 
} 

//And here is the implementation 

public IDatabase LoadDatabaseFromUserInput(){ 

    IDatabase database = null; 
    userHasGivenUpAndQuit = false; 

    //Do looping close to the control (in this case the user) 
    do{ 
     try{ 
      //Wait for user input 
      GetUserInput(); 

      //Check user input for validity 
      CheckConfigFile(); 
      CheckDatabaseConnection(); 

      //This line shouldn't fail, but if it does we are 
      // going to let it bubble up to the next layer because 
      // we don't know what just happened 
      database = LoadDatabaseFromSettings(); 
     } 
     catch(ConfigFileException ex){ 
      Logger.Warning(ex); 
      DisplayUserFriendlyMessage(ex); 
     } 
     catch(CouldNotConnectToDatabaseException ex){ 
      Logger.Warning(ex); 
      DisplayUserFriendlyMessage(ex); 
     } 
     finally{ 
      //Clean up any resources here 
     } 
    }while(database != null); 
} 

Ora ovviamente non ho idea di cosa stia cercando di fare la tua applicazione, e questo non è certamente un esempio di produzione. Spero che tu abbia un'idea generale. Ristruttura il programma in modo da poter evitare eventuali interruzioni inutili nel flusso dell'applicazione.

Cheers, Josh

Answer 2

Personalmente, avrei questo in un metodo separato che restituisce un codice di stato di successo o fallimento. Quindi, nel codice che chiamerebbe questo metodo, posso avere un numero magico di volte che continuerei a provare questo finché il codice di stato è "Success". Non mi piace usare try/catch per il controllo del flusso.

Answer 3

No, non va bene: http://xkcd.com/292/

Answer 4

forse im manca qualcosa, ma Perchè non basta usare un ciclo while? questo ti darà lo stesso ciclo per sempre se hai una funzionalità di eccezione (che è un codice errato) fornita dal tuo codice.

IDatabase database = null; 

while(database == null){ 
    try 
    { 
     database = databaseLoader.LoadDatabase(); 
    } 
    catch(Exception e) 
    { 
     var connector = _userInteractor.GetDatabaseConnector(); 
     if(connector == null) 
       throw new ConfigException("Could not load the database specified in your config file."); 
     databaseLoader = DatabaseLoaderFacade.GetDatabaseLoader(connector); 
     //just in case?? 
     database = null; 
    } 
} 

se si deve utilizzare goto nel proprio codice normale, manca il flusso logico. che puoi ottenere usando costrutti standard, se, mentre, per ecc.

Answer 5

È chiaro? Non proprio. Quello che in realtà vuoi fare, credo, è innanzitutto provare a caricare il database e poi, se non funziona, prova a caricarlo in un modo diverso. È giusto? Scriviamo il codice in questo modo.

IDatabase loadedDatabase = null; 

// first try 
try 
{ 
    loadedDatabase = databaseLoader.LoadDatabase(); 
} 
catch(Exception e) { } // THIS IS BAD DON'T DO THIS 

// second try 
if(loadedDatabase == null) 
{ 
    var connector = _userInteractor.GetDatabaseConnector(); 
    if(connector == null) 
     throw new ConfigException("Could not load the database specified in your config file."); 
    databaseLoader = DatabaseLoaderFacade.GetDatabaseLoader(connector); 
    loadedDatabase = databaseLoader.LoadDatabase() 
} 

Questo illustra più chiaramente ciò che si sta effettivamente facendo. Come bonus aggiuntivo, altri programmatori non ti tireranno fuori gli occhi. :)

NOTA: quasi certamente non vuoi prendere l'eccezione. È probabile che si tratti di un'eccezione più specifica che preferiresti catturare. Ciò potrebbe anche prendere TheComputerIsOnFireException, dopo di che non vale la pena riprovare.

Answer 6

In una nota a margine, penso che ci sia un potenziale per un ciclo infinito se si ottiene sempre un'eccezione.

Tecnicamente non c'è niente di sbagliato nella struttura goto, ma per me, opterei per l'utilizzo di un ciclo while. Qualcosa di simile:

IDatabase database = null; 

bool bSuccess = false; 
int iTries = 0 
while (!bSuccess) // or while (database == null) 
{ 
    try 
    { 
     iTries++; 
     database = databaseLoader.LoadDatabase(); 
     bSuccess = true; 
    } 
    catch(DatabaseLoaderException e) 
    { 
     //Avoid an endless loop 
     if (iTries > 10) 
      throw e; 

     var connector = _userInteractor.GetDatabaseConnector(); 
     if(connector == null) 
      throw new ConfigException("Could not load the database specified in your config file."); 
     databaseLoader = DatabaseLoaderFacade.GetDatabaseLoader(connector); 
    } 
} 

Answer 7