Pourquoi une méthode ne devrait-elle pas lancer plusieurs types d'exceptions vérifiées?

Nous utilisons SonarQube pour analyser notre code Java et cette règle (définie sur critique) est la suivante:

Les méthodes publiques doivent renvoyer au plus une exception cochée

L'utilisation d'exceptions vérifiées oblige les appelants de méthode à traiter les erreurs, soit en les propageant, soit en les gérant. Cela fait de ces exceptions une partie intégrante de l'API de la méthode.

Pour que la complexité des appels reste raisonnable, les méthodes ne doivent pas lancer plus d'un type d'exception cochée. "

Un autre morceau de Sonar a ceci :

Les méthodes publiques doivent renvoyer au plus une exception cochée

L'utilisation d'exceptions vérifiées oblige les appelants de méthode à traiter les erreurs, soit en les propageant, soit en les gérant. Cela fait de ces exceptions une partie intégrante de l'API de la méthode.

Pour que la complexité des appels reste raisonnable, les méthodes ne doivent pas lancer plus d'un type d'exception cochée.

Le code suivant:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

devrait être reformulé dans:

public void delete() throws SomeApplicationLevelException {  // Compliant
    /* ... */
}

Les méthodes de substitution ne sont pas vérifiées par cette règle et sont autorisées à générer plusieurs exceptions vérifiées.

Je n'ai jamais rencontré cette règle / recommandation dans mes lectures sur la gestion des exceptions et j'ai essayé de trouver des normes, des discussions, etc. sur le sujet. La seule chose que j’ai trouvée est celle-ci dans CodeRach: combien d’exceptions une méthode devrait-elle au maximum être levée ?

Est-ce une norme bien acceptée?

Permet de considérer la situation où vous avez le code fourni de:

public void delete() throws IOException, SQLException {      // Non-Compliant
  /* ... */
}

Le danger ici est que le code que vous écrivez pour appeler delete()ressemblera à ceci :

try {
  foo.delete()
} catch (Exception e) {
  /* ... */
}

C'est mauvais aussi. Et il sera attrapé avec une autre règle qui drapeaux attrapant la classe de base Exception.

La clé est de ne pas écrire de code qui vous donne envie d'écrire du mauvais code ailleurs.

La règle que vous rencontrez est plutôt commune. Checkstyle l' a dans ses règles de conception:

ThrowsCount

Limite le nombre d'instructions au nombre spécifié (1 par défaut).

Justification: les exceptions font partie de l'interface d'une méthode. Déclarer une méthode générant trop d'exceptions ayant une racine différente rend la gestion des exceptions fastidieuse et conduit à des pratiques de programmation médiocres telles que l'écriture de code tel que catch (Exception ex). Cette vérification oblige les développeurs à placer les exceptions dans une hiérarchie telle que, dans le cas le plus simple, un seul type d'exception doit être vérifié par un appelant, mais toute sous-classe peut être interceptée spécifiquement si nécessaire.

Ceci décrit précisément le problème et le problème et pourquoi vous ne devriez pas le faire. C’est une norme bien acceptée que de nombreux outils d’analyse statique vont identifier et signaler.

Et bien que vous puissiez le faire en fonction de la conception de la langue, et il peut arriver que ce soit la bonne chose à faire, c'est quelque chose que vous devriez voir et que vous devriez immédiatement dire "pourquoi je fais ça?" Cela peut être acceptable pour un code interne où tout le monde est assez discipliné pour ne jamais le faire catch (Exception e) {}, mais le plus souvent, j'ai vu des gens prendre des raccourcis, en particulier dans des situations internes.

Ne faites pas que les gens qui utilisent votre classe veuillent écrire du mauvais code.

Je devrais souligner que l'importance de cela est réduite avec Java SE 7 et les versions ultérieures, car une seule instruction catch peut intercepter plusieurs exceptions ( capture de plusieurs types d'exceptions et retransmission d'exceptions avec une vérification de type améliorée à partir d'Oracle).

Avec Java 6 et les versions antérieures, vous auriez un code ressemblant à ceci:

public void delete() throws IOException, SQLException {
  /* ... */
}

et

try {
  foo.delete()
} catch (IOException ex) {
     logger.log(ex);
     throw ex;
} catch (SQLException ex) {
     logger.log(ex);
     throw ex;
}

ou

try {
    foo.delete()
} catch (Exception ex) {
    logger.log(ex);
    throw ex;
}

Aucune de ces options avec Java 6 n'est idéale. La première approche viole DRY . Plusieurs blocs faisant la même chose, encore et encore - une fois pour chaque exception. Vous souhaitez enregistrer l'exception et la renvoyer? D'accord. Les mêmes lignes de code pour chaque exception.

La deuxième option est pire pour plusieurs raisons. Premièrement, cela signifie que vous attrapez toutes les exceptions. Le pointeur Null se fait attraper là (et il ne devrait pas). De plus, vous renvoyez un mot, Exceptionce qui signifie que la signature de la méthode serait une source de deleteSomething() throws Exceptiondégâts supplémentaires dans la pile, car les utilisateurs de votre code sont désormais contraints de le faire catch(Exception e).

Avec Java 7, ce n’est pas aussi important car vous pouvez faire:

catch (IOException|SQLException ex) {
    logger.log(ex);
    throw ex;
}

De plus, le type de vérification si l' on n'Attrapez les types des exceptions étant jeté:

public void rethrowException(String exceptionName)
throws IOException, SQLException {
    try {
        foo.delete();
    } catch (Exception e) {
        throw e;
    }
}

Le vérificateur de type reconnaîtra qu'il nee peut s'agir que de types ou . Je ne suis toujours pas très enthousiaste à propos de l'utilisation de ce style, mais cela ne cause pas le même code que sous Java 6 (dans lequel la signature de la méthode vous obligerait à utiliser la superclasse qui étend les exceptions).IOExceptionSQLException

Malgré tous ces changements, de nombreux outils d'analyse statiques (Sonar, PMD, Checkstyle) appliquent toujours les guides de style Java 6. Ce n'est pas une mauvaise chose. J'ai tendance à être d'accord avec un avertissement selon lequel celles-ci doivent toujours être appliquées, mais vous pouvez changer la priorité en priorité majeure ou mineure en fonction de la manière dont votre équipe les hiérarchise.

Si les exceptions doivent cocher ou décocher ... qui est une question de g r e un t débat que l' on peut facilement trouver d' innombrables messages de blog qui se place de chaque côté de l'argument. Toutefois, si vous utilisez des exceptions vérifiées, vous devriez probablement éviter de lancer plusieurs types, du moins sous Java 6.

La raison pour laquelle, idéalement, vous voudriez ne lever qu'un seul type d'exception est parce que cela enfreint probablement les principes de responsabilité unique et d' inversion de dépendance . Utilisons un exemple pour démontrer.

Supposons que nous ayons une méthode qui extrait les données de la persistance et que cette persistance est un ensemble de fichiers. Comme nous traitons de fichiers, nous pouvons avoir FileNotFoundException:

public String getData(int id) throws FileNotFoundException

Nous modifions maintenant les exigences et nos données proviennent d'une base de données. Au lieu d'un FileNotFoundException(puisque nous ne traitons pas de fichiers), nous lançons maintenant un SQLException:

public String getData(int id) throws SQLException

Nous devrions maintenant parcourir tout le code qui utilise notre méthode et changer l'exception à vérifier, sinon le code ne sera pas compilé. Si notre méthode est appelée de loin, il peut en être beaucoup de changer / d’en changer d’autres. Cela prend beaucoup de temps et les gens ne vont pas être heureux.

L'inversion de dépendance indique que nous ne devrions vraiment pas lancer l'une ou l'autre de ces exceptions, car elles exposent les détails de l'implémentation interne que nous travaillons à encapsuler. Le code d'appel a besoin de savoir quel type de persistance nous utilisons, alors qu'il faut juste s'inquiéter de savoir si l'enregistrement peut être récupéré. Au lieu de cela, nous devrions lancer une exception qui transmet l'erreur au même niveau d'abstraction que nous exposons via notre API:

public String getData(int id) throws InvalidRecordException

Maintenant, si nous changeons l'implémentation interne, nous pouvons simplement envelopper cette exception dans un InvalidRecordExceptionet le transmettre (ou ne pas l'envelopper, et simplement en lancer une nouvelle InvalidRecordException). Le code externe ne sait pas ou se soucie du type de persistance utilisé. Tout est encapsulé.

En ce qui concerne la responsabilité unique, nous devons penser au code qui jette plusieurs exceptions non liées. Disons que nous avons la méthode suivante:

public Record parseFile(String filename) throws IOException, ParseException

Que pouvons-nous dire à propos de cette méthode? Nous pouvons simplement dire à partir de la signature que le fichier est ouvert et analysé. Lorsque nous voyons une conjonction, comme "et" ou "ou" dans la description d'une méthode, nous savons qu'elle fait plus d'une chose. il a plus d'une responsabilité . Les méthodes comportant plusieurs responsabilités sont difficiles à gérer car elles peuvent changer si l'une ou l'autre de ces responsabilités change. Au lieu de cela, nous devrions casser les méthodes pour qu'elles n'aient qu'une seule responsabilité:

public String readFile(String filename) throws IOException
public Record parse(String data) throws ParseException

Nous avons retiré la responsabilité de la lecture du fichier de la responsabilité de l'analyse des données. Un effet secondaire de cela est que nous pouvons maintenant transmettre n'importe quelle donnée String aux données d'analyse de n'importe quelle source: en mémoire, fichier, réseau, etc. Nous pouvons également tester parseplus facilement maintenant car nous n'avons pas besoin d'un fichier sur le disque. faire des tests contre.

Parfois, il y a vraiment deux (ou plus) exceptions que nous pouvons tirer d'une méthode, mais si nous nous en tenons à SRP et DIP, les moments où nous rencontrons cette situation deviennent plus rares.

Je me souviens avoir joué un peu avec cela lorsque je jouais avec Java il y a quelque temps, mais je n'étais pas vraiment conscient de la distinction entre coché et décoché jusqu'à ce que je lise votre question. J'ai trouvé cet article sur Google assez rapidement, et il entre dans une partie de la controverse apparente:

http://tutorials.jenkov.com/java-exception-handling/checked-or-unchecked-exceptions.html

Cela étant dit, l’un des problèmes mentionnés par les experts en ce qui concerne les exceptions vérifiées est que (et j’en ai personnellement parlé depuis le début avec Java) si vous continuez d’ajouter de nombreuses exceptions vérifiées aux throwsclauses dans vos déclarations de méthode, pas seulement. devez-vous mettre beaucoup plus de code standard pour le prendre en charge lorsque vous passez à des méthodes de niveau supérieur, mais cela crée également un désordre plus important et annule la compatibilité lorsque vous essayez d'introduire davantage de types d'exception dans les méthodes de niveau inférieur. Si vous ajoutez un type d'exception coché à une méthode de niveau inférieur, vous devez alors réexaminer votre code et ajuster plusieurs autres déclarations de méthode.

L'un des points d'atténuation mentionnés dans l'article - et l'auteur n'aimait pas cela personnellement - consistait à créer une exception de classe de base, à limiter vos throwsclauses à une utilisation exclusive, puis à n'en créer que des sous-classes en interne. De cette façon, vous pouvez créer de nouveaux types d'exceptions cochées sans avoir à parcourir tout votre code.

L’auteur de cet article n’a peut-être pas aimé cela beaucoup, mais mon expérience personnelle me semble parfaitement logique (surtout si vous pouvez consulter toutes les sous-classes de toute façon), et je parie que c’est pourquoi le conseil qui vous est donné est de: plafonnez tout à un type d'exception coché chacun. De plus, les conseils que vous avez mentionnés autorisent plusieurs types d'exceptions vérifiées dans les méthodes non publiques, ce qui est parfaitement logique si tel est leur motif (ou même autrement). Si c'est juste une méthode privée ou quelque chose de similaire, de toute façon, vous n'allez pas parcourir la moitié de votre base de code quand vous changez une petite chose.

Vous avez en grande partie demandé s'il s'agissait d'une norme acceptée, mais entre vos recherches que vous avez mentionnées, cet article raisonnablement réfléchi et le fait que vous parliez d'une expérience de programmation personnelle, il ne semble certainement pas se démarquer.

Lancer plusieurs exceptions vérifiées est logique lorsqu'il y a plusieurs choses raisonnables à faire.

Par exemple, disons que vous avez une méthode

public void doSomething(Credentials cred, Work work) 
    throws CredentialsRequiredException, TryAgainLaterException{...}

cela viole la règle d'exception de pne, mais est logique.

Malheureusement, ce qui se passe habituellement sont des méthodes comme

void doSomething() 
    throws IOException, JAXBException,SQLException,MyException {...}

Dans ce cas, l'appelant a peu de chances de faire quelque chose de spécifique en fonction du type d'exception. Donc, si nous voulons le pousser à se rendre compte que ces méthodes peuvent et vont parfois mal tourner, lancer simplement quelque chose de SombreMightGoWrongException est enogh et meilleur.

Par conséquent, réglez au plus une exception cochée.

Mais si votre projet utilise une conception comportant plusieurs exceptions vérifiées signalisées, cette règle ne doit pas s'appliquer.

Sidenote: Quelque chose peut vraiment mal se passer presque partout, alors on peut penser à utiliser? étend RuntimeException, mais il y a une différence entre "nous faisons tous des erreurs" et "ce système parle en externe et il sera parfois en panne, résoudrez-le".