Est d'avoir une instruction de retour juste pour satisfaire la syntaxe d'une mauvaise pratique?

Question

Considérons le code suivant:

public Object getClone(Cloneable a) throws TotallyFooException {

    if (a == null) {
        throw new TotallyFooException();
    }
    else {
        try {
            return a.clone();
        } catch (CloneNotSupportedException e) {
            e.printStackTrace();
        }
    }

    //cant be reached, in for syntax
    return null;

}

L' return null; est nécessaire depuis une exception peut être pris, cependant, dans de tels cas, puisque nous avons déjà vérifié si il était nul (et supposons que nous connaissons la classe que nous appelons prend en charge le clonage), donc nous savons que le rapport d'essai ne manquera jamais.

Est-ce une mauvaise pratique à mettre dans le return à la fin juste pour satisfaire la syntaxe et d'éviter les erreurs de compilation (avec un commentaire expliquant qu'il ne sera pas atteint), ou est-il une meilleure façon de coder quelque chose comme cela pour que le retour supplémentaire déclaration est inutile?

Answer 1

D'une manière plus claire, sans un supplément d'instruction return est comme suit. Je ne voudrais pas attraper CloneNotSupportedException , soit, mais le laisser aller à l'appelant.

if (a != null) {
    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        e.printStackTrace();
    }
}
throw new TotallyFooException();

Il est presque toujours possible de jouer avec la commande pour plus straight-forward de syntaxe que ce que vous devez initialement.

Answer 2

Il peut certainement être atteint. Notez que vous avez l'impression que la stacktrace dans l' catch de la clause.

Dans le scénario où l' a != null et il y aura une exception, l' return null va être atteint. Vous pouvez supprimer cette déclaration et de la remplacer par throw new TotallyFooException();.

En général^*, si null est valable résultat d'une méthode (c'est à dire que l'utilisateur s'attend à ce qu'elle et cela signifie quelque chose) puis de le retourner comme un signal pour les "données non trouvé" ou d'exception qui s'est passé n'est pas une bonne idée. Sinon, je ne vois pas de problème, pourquoi vous ne devriez pas rendement null.

Prenez l'exemple de l' Scanner#ioException méthode:

Renvoie l' IOException dernière levée par ce Scanner sous-jacente Lisible. Cette méthode renvoie null si aucune exception de ce type n'existe.

Dans ce cas, la valeur retournée null a un sens évident, lorsque j'utilise la méthode, je peux être sûr que j'ai eu null seulement parce qu'il n'y avait pas une telle exception et non pas parce que la méthode a essayé de faire quelque chose et il a échoué.

^*Notez que, parfois, vous ne voulez renvoyer null , même lorsque le sens est ambigu. Par exemple, l' HashMap#get:

Une valeur de retour nulle ne signifie pas nécessairement que la carte ne contient pas de cartographie pour la clé; il est également possible que la carte explicitement les cartes de la clé null. L' containsKey opération peut être utilisé pour distinguer ces deux cas.

Dans ce cas - null peut indiquer que la valeur null a été retrouvé et retourné ou que la table de hachage ne contient pas la clé demandée.

Answer 3

Est-ce une mauvaise pratique à mettre dans le return à la fin juste pour satisfaire la syntaxe et d'éviter les erreurs de compilation (avec un commentaire expliquant qu'il ne sera pas atteint)

Je pense que return null est une mauvaise pratique pour le terminus d'une inaccessible branche. Il est préférable de jeter un RuntimeException (AssertionError serait également acceptable) pour faire de cette ligne a quelque chose de très mauvais, et l'application est dans un état inconnu. La plupart, comme c'est (comme ci-dessus), car le développeur a manqué quelque chose (les Objets peuvent être aucun nulles et non-clonable).

Je probablement ne pas utiliser InternalError sauf si je suis très certain que le code est inaccessible (par exemple, après l' System.exit()) comme il est plus probable que je fais une erreur de la machine virtuelle.

J'avais seulement utiliser une exception personnalisée (comme TotallyFooException) si l'obtention de cette "inaccessible ligne" signifie la même chose que n'importe où ailleurs vous jeter cette exception.

Answer 4

Vous avez pris l' CloneNotSupportedException ce qui signifie que votre code peut s'en occuper. Mais après vous l'attrapez, vous n'avez absolument aucune idée de quoi faire lorsque vous atteignez la fin de la fonction, ce qui implique que vous ne pouvais pas gérer. Donc, vous avez raison, c'est une odeur de code dans ce cas, et à mon avis signifie que vous ne devriez pas avoir attrapé CloneNotSupportedException.

Answer 5

Je préfère utiliser Objects.requireNonNull() de vérifier si le Paramètre a n'est pas nulle. Il est donc clair lorsque vous lisez le code que le paramètre ne doit pas être null.

Et pour éviter checked Exceptions je re jeter l' CloneNotSupportedException comme RuntimeException.

Pour les deux, vous pourriez ajouter un joli texte avec l'intention pourquoi cela ne devrait pas arriver ou être le cas.

public Object getClone(Object a) {

    Objects.requireNonNull(a);

    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        throw new IllegalArgumentException(e);
    }

}