Ist dies eine eindeutige Verwendung von goto?

Question

Ich frage mich nur, ob dies als eine klare Verwendung von goto in C# betrachtet wird:

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;
}

Ich denke, das ist in Ordnung, denn der Ausschnitt ist klein und sollte sinnvoll sein. Gibt es eine andere Art und Weise Menschen in der Regel von Fehlern wie diese erholen, wenn Sie den Vorgang nach der Behandlung der Ausnahme wiederholen möchten?

Bearbeiten: Das war schnell. Um ein paar Fragen zu beantworten und die Dinge ein wenig zu klären - dies ist Teil eines Prozesses, der im Wesentlichen eine Konvertierung von einer anderen Art von Projekt ist. Der Aufruf _userInteractor.GetDatabaseConnector() ist der Teil, der bestimmt, ob der Benutzer es erneut versuchen möchte (möglicherweise mit einer anderen Datenbank als der in der Konfiguration, aus der er lädt). Wenn er null zurückgibt, wurde keine neue Datenbankverbindung angegeben und der Vorgang sollte vollständig fehlschlagen.

Ich habe keine Ahnung, warum ich nicht daran gedacht habe, eine while-Schleife zu verwenden. Es ist wohl schon zu spät für 17 Uhr.

Bearbeiten 2: Ich habe mir die LoadDatabase()-Methode angeschaut, und es wird ein DatabaseLoaderException wenn es scheitert. Ich habe den Code oben aktualisiert, um diese Ausnahme anstelle von Exception abzufangen.

Bearbeiten 3: Der allgemeine Konsens scheint zu sein, dass

• Die Verwendung von goto ist hier nicht notwendig - eine while-Schleife reicht völlig aus.
• Die Verwendung von Ausnahmen wie dieser ist keine gute Idee - ich bin mir allerdings nicht sicher, womit ich sie ersetzen könnte.

Answer 1

Gibt es einen anderen Weg, wie Menschen Fehler wie diesen zu beheben, wenn man den Vorgang wiederholen möchte, nachdem Behandlung der Ausnahme?

Ja, im Aufrufcode. Lassen Sie den Aufrufer dieser Methode entscheiden, ob er die Logik erneut versuchen muss oder nicht.

UPDATE:

Zur Verdeutlichung: Sie sollten Ausnahmen nur dann abfangen, wenn Sie sie tatsächlich behandeln können. Ihr Code sagt im Grunde genommen:

"Ich habe keine Ahnung, was passiert ist, aber was immer ich getan habe, hat alles in die Luft gejagt. in die Luft gejagt... also lasst es uns noch einmal tun."

Fangen Sie bestimmte Fehler ab, die Sie beheben können, und lassen Sie den Rest an die nächste Schicht weiterleiten, damit er dort behandelt wird. Alle Ausnahmen, die es bis ganz nach oben schaffen, sind zu diesem Zeitpunkt echte Fehler.

UPDATE 2:

Ok, also anstatt eine eher langwierige Diskussion über die Kommentare fortzusetzen, werde ich mit einem Semi-Pseudo-Code-Beispiel erläutern.

Die allgemeine Idee ist, dass Sie den Code umstrukturieren müssen, um Tests durchzuführen und die Benutzererfahrung ein wenig besser zu gestalten.

//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); 
}

Ich habe natürlich keine Ahnung, was Ihre Anwendung zu tun versucht, und dies ist ganz sicher kein Produktionsbeispiel. Ich hoffe, Sie haben den Grundgedanken verstanden. Strukturieren Sie das Programm so um, dass Sie unnötige Unterbrechungen im Anwendungsfluss vermeiden können.

Zum Wohl, Josh

Answer 2

Vielleicht im etwas vermissen, aber warum kann man nicht einfach eine while-Schleife verwenden? dies wird Ihnen die gleiche Schleife für immer, wenn Sie eine Ausnahme (die schlechte Code ist) Funktionalität, die Ihren Code gibt.

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;
   }
 }

Wenn Sie goto in Ihrem normalen Code verwenden müssen, fehlt Ihnen der logische Fluss, den Sie mit Standardkonstrukten wie if, while, for usw. erreichen können.

Answer 3

Ich persönlich würde dies in einer separaten Methode durchführen, die einen Statuscode für Erfolg oder Misserfolg zurückgibt. Dann kann ich in dem Code, der diese Methode aufruft, eine magische Anzahl von Versuchen festlegen, bis der Statuscode "Erfolg" lautet. Ich mag es einfach nicht, try/catch für den Kontrollfluss zu verwenden.

Answer 4

Ist das klar? Nicht wirklich. Ich denke, Sie sollten zuerst versuchen, die Datenbank zu laden, und dann, wenn das nicht funktioniert hat, versuchen, sie auf eine andere Weise zu laden. Ist das richtig? Lassen Sie uns den Code auf diese Weise schreiben.

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()
}

So wird deutlicher, was Sie tatsächlich tun. Als zusätzlichen Bonus werden Ihnen andere Programmierer nicht die Augen ausstechen :)

HINWEIS: Sie wollen mit Sicherheit keine Ausnahmegenehmigung einholen. Es gibt wahrscheinlich eine spezifischere Ausnahme, die Sie lieber abfangen würden. Dies würde auch TheComputerIsOnFireException abfangen, wonach es sich nicht wirklich lohnt, den Versuch zu wiederholen.

Answer 5

Nein, es ist nicht in Ordnung: http://xkcd.com/292/