2010-11-14 7 views
5

En regardant ma méthode, je ne suis pas sûr d'avoir à utiliser trois blocs try/catch séparés. Dois-je utiliser le seul pour toute la méthode? Qu'est-ce qu'une bonne pratique?Si plusieurs blocs Try/Catch d'une méthode doivent être combinés

public void save(Object object, File file) {  

     BufferedWriter writter = null; 

     try { 
      writter = new BufferedWriter(new FileWriter(file)); 
     } catch (IOException e) { 
      e.printStackTrace(); 
     } 

     Dictionary dictionary = (Dictionary)object; 
     ArrayList<Card> cardList = dictionary.getCardList(); 
     for (Card card: cardList) { 
      String line = card.getForeignWord() + "/" + card.getNativeWord(); 
      try { 
       writter.write(line); 
      } catch (IOException e) { 
       e.printStackTrace(); 
      } 
     } 
     try { 
      writter.flush(); 
      writter.close(); 
     } catch (IOException e) { 
      e.printStackTrace(); 
     } 
    } 

Répondre

11

Vous ne voulez certainement pas trois blocs séparés avec le code tel qu'il est. Dans le premier bloc, vous rencontrez une erreur lors de la configuration writer, mais dans les blocs suivants, vous êtes en utilisantwriter, ce qui n'a aucun sens si le premier bloc a échoué. Vous finirez par lancer un NullPointerException lorsqu'une erreur d'E/S se produit   — n'est pas idéal. :-)

Il y a beaucoup de place pour le style dans ce genre de choses, mais voici une réinterprétation assez standard de votre fonction. Il utilise seulement deux blocs (bien que vous pouvez choisir de rajouter un troisième, voir les commentaires dans le code):

public void save(Object object, File file) {  

    BufferedWriter writter = null; 

    try { 
     writter = new BufferedWriter(new FileWriter(file)); 

     Dictionary dictionary = (Dictionary)object; 
     ArrayList<Card> cardList = dictionary.getCardList(); 
     for (Card card: cardList) { 
      String line = card.getForeignWord() + "/" + card.getNativeWord(); 
      writter.write(line); // <== I removed the block around this, on 
           // the assumption that if writing one card fails, 
           // you want the whole operation to fail. If you 
           // just want to ignore it, you would put back 
           // the block. 
     } 

     writter.flush(); // <== This is unnecessary, `close` will flush 
     writter.close(); 
     writter = null; // <== null `writter` when done with it, as a flag 
    } catch (IOException e) { 
     e.printStackTrace(); // <== Usually want to do something more useful with this 
    } finally { 
     // Handle the case where something failed that you *didn't* catch 
     if (writter != null) { 
      try { 
       writter.close(); 
       writter = null; 
      } catch (Exception e2) { 
      } 
     } 
    } 
} 

Note sur le bloc finally: Ici, vous pouvez manutentionneront le cas normal (dans ce cas, writter sera null), ou vous pourriez gérer une exception que vous n'avez pas détectée (ce qui n'est pas inhabituel, l'un des principaux points d'exceptions est de gérer ce qui est approprié à ce niveau et de passer toute autre chose à l'appelant) . Si writter est !null, fermez-le. Et quand vous le fermez, manger toute exception qui se produit ou vous obscurcir l'original. (J'ai des fonctions utilitaires pour fermer des choses tout en mangeant une exception, pour exactement cette situation.Pour moi, cela pourrait être writter = Utils.silentClose(writter); [silentClose retourne toujours null]). Maintenant, dans ce code, vous ne pouvez pas vous attendre à d'autres exceptions, mais A) Vous pouvez changer cela plus tard, et B) RuntimeException s peut se produire à tout moment. Il vaut mieux s'habituer à utiliser le motif.

+1

Merci pour une si bonne explication. – Eugene

+0

@AndroidNoob: Pas de soucis! Content que cela ait aidé. –

+0

@ T.J. Crowder pourquoi fermez-vous 'writer' dans le bloc' try'? N'est-ce pas redondant? –

1

C'est une question de goût - il n'y a pas de bonne réponse.

Personnellement, je préfère un bloc try/catch. Si j'en vois beaucoup, je commence à m'inquiéter que ma méthode en fasse trop et qu'elle devienne trop grosse. C'est un signe qu'il pourrait être temps de le diviser en plus petites méthodes.

Voilà comment j'écrire cette méthode:

public void save(Object object, File file) throws IOException 
{ 

    BufferedWriter writer = null; 

    try { 
    writer = new BufferedWriter(new FileWriter(file)); 

    Dictionary dictionary = (Dictionary) object; 
    ArrayList<Card> cardList = dictionary.getCardList(); 
    for (Card card : cardList) 
    { 
     String line = card.getForeignWord() + "/" + card.getNativeWord(); 
     writer.write(line); 
    } 
    } 
    finally 
    { 
     IOUtils.close(writer); 
    }  
} 

j'aurais une classe IOUtils qui intégrerait la fermeture correcte des flux, les lecteurs et écrivains. Vous allez écrire ce code encore et encore. Je ferais un one-liner comme méthode statique dans une classe - ou j'utiliserais le JAR Apache Commons IO Utils si cela ne me dérangeait pas une autre dépendance.

+2

Il n'y a en effet pas de bonne réponse, mais à mon avis, il ne s'agit pas seulement de goût – Peter

+0

D'accord, Peter. Il y a la question d'essayer/finalement, etc. – duffymo

1

Ce qui est bon dépend de ce que votre code est destiné à faire. Par exemple, le second try/catch est à l'intérieur d'une boucle, ce qui signifie qu'il traitera tous les card s dans la liste, même si le traitement de l'un est échoué. Si vous utilisez un try/catch pour l'ensemble de la méthode, ce comportement sera modifié. Utilisez un try/catch si votre algo le permet.

0

Dans le cas particulier de I/OI suggère que vous regardiez quelques-unes des bibliothèques courantes disponibles comme les communes de Guava et de Jakarta. Ils ont quelques bons scripts pour effectuer des fermetures silencieuses.

E.g. Commons IO IOUtils

Ceux-ci peuvent nettoyer votre code tout à fait un peu

0

Je préfère écrire deux blocs d'essai, un pour le enfin et l'autre pour les prises. En raison de la façon dont il est écrit, vous ne définissez jamais null à null, et vous ne devez pas non plus vérifier par rapport à null lors de la fermeture. Le seul problème est une exception alors que la fermeture cacherait une exception pendant le traitement, mais c'est seulement un problème si vous voulez gérer les deux exceptions différemment.

public void save(Object object, File file) { 
    try { 
     BufferedWriter writter = new BufferedWriter(new FileWriter(file)); 
     try { 
      Dictionary dictionary = (Dictionary)object; 
      ArrayList<Card> cardList = dictionary.getCardList(); 
      for (Card card: cardList) { 
       String line = card.getForeignWord() + "/" + card.getNativeWord(); 
       writter.write(line); 
      } 
     } finally { 
      writter.flush(); 
      writter.close(); 
     } 
    } catch (IOException e) { 
     e.printStackTrace(); //Or something more useful 
    } 
} 
Questions connexes