Je me demande simplement si cela est considéré comme une utilisation claire de goto en 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;
}
7 Réponses :
Personnellement, j'aurais ceci dans une méthode distincte qui renvoie un code de statut de succès ou d'échec. Ensuite, dans le code qui appellerait cette méthode, je peux avoir un nombre magique de fois que je continuerais à essayer jusqu'à ce que le code d'état soit "succès". Je n'aime tout simplement pas utiliser Essayer / Catch pour le flux de contrôle.
Le nombre de fois magique est critique: le code de l'OP ferait une boucle infinie si l'erreur persiste (ce qui est probablement assez probable si une erreur se produit du tout).
Vous pouvez envelopper votre chèque à un numéro magique autour d'une instruction IF, et si votre numéro magique est 0 ou -1, alors ne vous inquiétez pas pour toujours arrêter la boucle avant de réussir.
Y a-t-il un autre moyen des gens habituellement récupérer des erreurs comme celle-ci lorsque vous vouloir réessayer l'opération après Manipulation de l'exception? P>
Oui, dans le code d'appel. Laissez l'appelant de cette méthode décider si elles ont besoin de réessayer la logique ou non. P>
Mise à jour: strong> p>
Pour clarifier, vous ne devez attraper que si vous peut en fait les gérer. Votre code dit essentiellement: P>
"Je n'ai aucune idée de ce qui s'est passé, mais quoi que je faisais tout causé faire exploser ... alors laisse le refaire. " em> p> blockQuote>
attrape des erreurs spécifiques que vous pouvez récupérer à partir de, et laissez le reste à bulles jusqu'à la couche suivante à traiter. Toutes les exceptions qui rendent tout le chemin du haut représentent de véritables bugs à ce point. P>
update 2: strong> p>
OK, donc plutôt que de continuer Une longue discussion via les commentaires que je vais élaborer avec un exemple de code semi-pseudo exemple. P>
L'idée générale est que vous devez simplement restructurer le code afin d'effectuer des tests et de gérer l'expérience utilisateur un peu mieux. P>
//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); }
Ce serait mon approche normale, mais cela fait partie d'un processus de lot de tri, donc je ne peux pas simplement jeter et attendre une tentative. Le connecteur de base de données qu'elle commence par est extraite à partir d'un fichier de configuration, le traitement des erreurs donne à l'utilisateur une chance de récupérer à partir d'une erreur dans cette configuration. J'ai remplacé le attrape (exception e) avec attrape (DatabaseLoaderException e) pour le rendre plus spécifique.
@Jamie: Vous ne pouvez pas attraper, réessayer et vous attendre à un résultat réussi immédiatement - la boucle peut très bien être sans fin si une exception est survenue. Connectez-vous l'erreur, même l'envoyer par courrier électronique. Si vous venez de lancer l'erreur et d'abandonner, la prochaine fois que le lot est exécuté, il va toujours essayer à nouveau, mais plus tard - il y a plus d'une chance qu'il fonctionnera plus tard (avoir eu le temps de résoudre les erreurs entre les courses) au lieu d'un continu. boucle sur une erreur.
Regardez mon édition ci-dessus, j'ai mentionné cela. La boucle se terminera une fois que l'utilisateur cesse d'entrer dans de nouvelles informations de connexion de base de données -> ils peuvent l'annuler à n'importe quel stade. _Utilisateur d'interactor est une interface pour l'interface utilisateur et retournera NULL si l'utilisateur annule. Donc, ce ne sera pas une erreur continue.
Je pense que vous devez refacturer votre code certains. Le problème est que vous utilisez des exceptions pour gérer le flux d'applications. Les exceptions doivent être ... Eh bien ... exceptionnel. Si vous attendez quelque chose à faire, vous devriez alors tester pour cela dans votre code. Essayez de réorganiser les choses afin que vous n'ayez pas à envelopper toute cette logique dans une prise.
Qu'est-ce qui se qualifie comme exceptionnel? Le chargeur de base de données lancera s'il ne peut pas charger de la base de données, et l'exception contient des informations sur l'erreur. Cela peut être n'importe quoi d'une erreur de connexion à la base de données, à une erreur dans le schéma. Je pouvais attraper des sous-types d'exception particulières et faire quelque chose de différent sur la base de chaque exception, mais je voudrais juste signaler les informations d'exception à l'utilisateur et les laisser décider s'ils veulent modifier des bases de données ou annuler et corriger les choses.
Exactement mon point cependant. Si l'utilisateur est vraiment en contrôle de ce qui se passe ici, ils devraient être informés de ce qui se passe. Pensez à SQL Management Studio ou à l'explorateur Visual Studio Server. Que se passe-t-il lorsque la connexion à la base de données échoue? Le message bulles jusqu'à la couche UI, puis l'utilisateur doit décider de la procédure à suivre. Je ne dis pas que votre code est fondamentalement brisé ou quoi que ce soit, mais il vous suffit de l'améliorer afin que l'appelant avec le plus d'informations puisse prendre une décision sur la manière de gérer l'exception. Ne cachez pas cette information.
Je comprends ce que tu veux dire maintenant. Cela fait beaucoup plus de sens que ce que je pensais. Merci.
Parfois, il peut être difficile de voir la forêt pour les arbres;) Il est toujours bon d'obtenir quelqu'un qui n'est pas proche du domaine du problème pour examiner votre code.
Non, ce n'est pas correct: http://xkcd.com/292/
Faire des déclarations de couverture sur les gotos est aussi mauvaise que d'utiliser sans distinction. Goto's sont une construction de programmation avancée à utiliser uniquement par des programmeurs avancés pour simplifier la structure de la méthode. L'utilisation en question par l'OP n'est pas appropriée, c'est certainement sûr (on a répondu pourquoi déjà), mais alors disant que les gotos sont toujours mauvaises, c'est aussi mauvais.
Hmmm ... argumentum ad xkcd est une erreur acceptable et excellente!
Drôle, mais pas réellement utile. -1
Peut-être que je manque quelque chose, mais pourquoi ne pouvez-vous pas simplement utiliser une boucle tandis que? Cela vous donnera la même boucle pour toujours si vous avez une exception (qui est une mauvaise coche) fonctionnalité que votre code donne.
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;
}
}
Ce ne sera pas, car il ne charge pas la même base de données pour toujours.
Si LoadDatabase () renvoie NULL et n'a pas échoué, cela ne sera pas?
Cette méthode ne renvoie pas NULL, mais que cette connaissance ne devrait pas être utilisée ici.
Si vous regardez le commentaire ci-dessus du code, vous remarquerez que je dis que le code ira éternellement, de même que le code d'origine de la question, la réponse est de faire avec l'utilisation de goto vs a while boucle, non si elle est Bon code (que j'ai dit que ce n'est pas)
Même si ce code est aussi mauvais que d'utiliser un goto, il obtient un peu mieux si vous avez utilisé une boucle réelle et rompez après la base de données = Databaseloader.loadDatabase ();
Au risque de commencer un argument, la pause n'est qu'une forme de goto aussi. Dans le code normal, vous n'avez jamais besoin de l'utiliser. (sauf dans une déclaration de cas)
Est-ce clair? Pas vraiment. Ce que vous voulez réellement faire, je pense, est d'abord essayer de charger la base de données, puis, si cela n'a pas fonctionné, essayez de le charger différent. Est-ce correct? Écrivons le code de cette façon.
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()
}
Ah, je vois maintenant que vous souhaitez continuer à demander à l'utilisateur une connexion de base de données différente chaque fois qu'il échoue, cela ne fonctionnera donc pas. Mais cela devrait mieux répondre à votre question: Non, votre construction avec le goto n'est pas claire. :) une boucle de temps aussi plus appropriée, comme d'autres l'ont noté.
sur une note latérale, je pense qu'il existe un potentiel de boucle sans fin si vous obtenez toujours une exception.
Techniquement, il n'y a rien de mal à votre structure goto, mais pour moi, je choisirais d'utiliser une boucle de temps à la place. . Quelque chose comme:
Pas besoin de voter une question légitime, car il a les mots goto dedans.
À l'OP: l'utilisation de goto ne doit pas être "claire". Il doit être "correct". Votre utilisation est claire, mais très incorrecte. Vous avez de bonnes réponses ici, elles ont-elles attention à eux :-)
Mais ce n'est pas des hommes que la question doit être évitée.