2010-06-15 9 views
8

Sto scrivendo un'app che si collega a un sito Web e ne legge una riga. Lo faccio in questo modo:Chiusura risorsa Java

try{ 
     URLConnection connection = new URL("www.example.com").openConnection(); 
     BufferedReader rd = new BufferedReader(new InputStreamReader(connection.getInputStream())); 
     String response = rd.readLine(); 
     rd.close(); 
    }catch (Exception e) { 
     //exception handling 
    } 

È buono? Voglio dire, chiudo BufferedReader nell'ultima riga, ma non chiudo InputStreamReader. Devo creare un InputStreamReader standalone da connection.getInputStream e un BufferedReader dall'InputStreamReader standalone, piuttosto che chiudere tutti i due lettori? Penso che sarà meglio mettere i metodi di chiusura nel blocco finally come questo:

InputStreamReader isr = null; 
BufferedReader br = null; 
try{ 
    URLConnection connection = new URL("www.example.com").openConnection(); 
    isr = new InputStreamReader(connection.getInputStream()); 
    br = new BufferedReader(isr); 
    String response = br.readLine(); 
}catch (Exception e) { 
    //exception handling 
}finally{ 
    br.close(); 
    isr.close(); 
} 

Ma è brutto, perché i metodi di chiusura possono gettare un'eccezione, quindi devo gestire o gettarlo.

Quale soluzione è migliore? O quale sarebbe la soluzione migliore?

+0

Il codice è in realtà ha un piccolo problema nella clausola finally. È possibile che 'isr' e' br' siano ancora 'null' nella clausola finally come' InputStreamReader' e che i costruttori 'BufferedReader' generino eccezioni. Dovresti cambiare la clausola finally in: 'finally {if (br! = Null) br.close(); if (isr! = null) isr.close(); } '. Questo non è ancora corretto perché 'br.close() 'potrebbe lanciare un'eccezione e quindi 'isr' non verrà chiuso in quel caso. La risposta di Andreas sembra essere il modo di seguire IMHO. – Behrang

+0

Non catturare mai "Eccezione" a meno che non si sappia perché –

risposta

6

L'idioma generale per l'acquisizione delle risorse e rilasciare in Java è:

final Resource resource = acquire(); 
try { 
    use(resource); 
} finally { 
    resource.release(); 
} 

Nota:

  • try dovrebbe seguire immediatamente l'acquisiscono. Questo significa che non puoi avvolgerlo nel decoratore e mantenere la sicurezza (e rimuovere gli spazi o mettere le cose su una sola riga non aiuta :).
  • Una versione per finally, altrimenti non sarà eccezionalmente sicura.
  • Evitare null, utilizzare final. Altrimenti avrete codice disordinato e potenziale per NPE.
  • Generalmente non è necessario chiudere il decoratore, a meno che non abbia un'ulteriore risorsa ad esso associata. Tuttavia, sarà generalmente necessario svuotare le uscite, ma evitarlo nel caso di eccezione.
  • L'eccezione deve essere passata al chiamante o catturata da un blocco try circostante (Java vi porta fuori strada qui).

o si può astrarre questa assurdità con lo Execute Around idiom, quindi non è necessario ripetere se stessi (basta scrivere un sacco di piastre).

+0

Come si può implementare un tale metodo di acquisizione da restituire, ad esempio "FileInputStream", senza che lancino un 'IOException'? È necessario avvolgere lo snippet di codice sopra all'interno di un altro blocco try {// il tuo snippet di codice} (IOException ioe) {...} 'e questo è altrettanto disordinato. Anche questo modello non funzionerà quando devi chiudere più risorse nel blocco finally in quanto il primo potrebbe generare un'eccezione e il prossimo non verrà affatto chiuso. – Behrang

+0

Se hai più risorse hai bisogno di più istruzioni 'try'-'finally'. E sì, le eccezioni dovrebbero essere gestite all'esterno, possibilmente al di fuori del metodo. –

2

Chiudere il BufferedReader è sufficiente - questo chiude anche il lettore sottostante.

Yishai posted a nice pattern per chiudere i flussi (la chiusura potrebbe generare un'altra eccezione).

1

penso che sarà meglio mettere i metodi di chiusura nel blocco finally

Sì, sempre. Perché potrebbe verificarsi un'eccezione e le risorse non vengono rilasciate/chiuse correttamente.

È sufficiente chiudere il lettore più esterno perché sarà responsabile della chiusura di eventuali lettori allegati.

Sì, è brutto ... per ora. Penso che ci siano piani per uno automatic resource management in Java.

2

È buono?Voglio dire, chiudo BufferedReader nell'ultima riga, ma non chiudo InputStreamReader.

parte il fatto che essa dovrebbe essere fatto nel finally (in modo che la chiusura è assicurata, anche in caso di un'eccezione), va bene. Le classi di I/O Java usano il pattern decoratore. La chiusura sarà delegata ai flussi sottostanti.

Ma è brutto, perché i metodi di chiusura possono generare eccezioni, quindi devo gestirlo o lanciarlo.

Quando la chiusura genera un'eccezione, spesso significa semplicemente che l'altro lato è stato chiuso o eliminato, che è completamente fuori dal vostro controllo. Puoi al massimo registro o ignorarlo. In una semplice applicazione lo ignorerei. In un'applicazione mission-critical lo registrerei, solo per essere sicuro.

In un dado, il codice può essere riscritto come:

BufferedReader br = null; 
try { 
    URLConnection connection = new URL("www.example.com").openConnection(); 
    br = new BufferedReader(new InputStreamReader(connection.getInputStream())); 
    String response = br.readLine(); 
}catch (Exception e) { 
    //exception handling 
}finally{ 
    if (br != null) try { br.close(); } catch (IOException ignore) {} 
} 

In Java 7 ci sarà la gestione automatica delle risorse che avrebbe reso il codice più conciso:

try (BufferedReader br = new InputStreamReader(new URL("www.example.com").openStream())) { 
    String response = br.readLine(); 
} catch (Exception e) { 
    //exception handling 
} 

Vedi anche :

2
BufferedReader br = null; 

si dichiara una variabile senza assegnarle (null non conta - si tratta di un incarico inutile in questo caso). Questo è un codice "odore" in Java (ref Effective Java; Code Complete per ulteriori informazioni sulla dichiarazione delle variabili).

}finally{ 
    br.close(); 
    isr.close(); 
} 

primo luogo, è solo bisogno di chiudere il più in alto decoratore flusso (br chiuderà isr). In secondo luogo, se br.close() ha generato un'eccezione, non si chiamerebbe isr.close(), quindi non si tratta di codice audio. In determinate condizioni di eccezione, il codice nasconderà l'eccezione di origine con uno NullPointerException.

isr = new InputStreamReader(connection.getInputStream()); 

Se il (certamente improbabile) caso in cui il costruttore InputStreamReader gettato ogni tipo di eccezione di runtime, il flusso dal collegamento non sarebbe chiuso.

Utilizzare l'interfaccia Closeable per ridurre la ridondanza.

Ecco come vorrei scrivere il codice:

URLConnection connection = new URL("www.example.com").openConnection(); 
InputStream in = connection.getInputStream(); 
Closeable resource = in; 
try { 
    InputStreamReader isr = new InputStreamReader(in); 
    resource = isr; 
    BufferedReader br = new BufferedReader(isr); 
    resource = br; 
    String response = br.readLine(); 
} finally { 
    resource.close(); 
} 

Nota che:

  • non importa che tipo di eccezione viene generata (runtime o controllata) o dove, il codice non perde stream resources
  • non esiste un blocco catch; le eccezioni dovrebbero essere passate fino a dove il codice può prendere una decisione ragionevole sulla gestione degli errori; se questo metodo era il posto giusto, che ci circondano tutto quanto sopra con try/catch

Qualche tempo fa, ho trascorso qualche tempo a pensare a come avoid leaking resources/data when things go wrong.

+0

@Bob - Ortogonale alla domanda che conosco, ma non sai chi sta copiando questa roba - non c'è una corretta gestione dei caratteri in questo codice - cioè il controllo del tipo di contenuto/codifica dei dati di ritorno. – McDowell

0

Non sono necessarie più istruzioni di chiusura per nessuno degli stream e dei lettori nidificati in java.io. È molto raro che alla fine sia necessario chiudere più di una cosa in una sola: la maggior parte dei costruttori può generare un'eccezione, quindi si cercherebbe di chiudere le cose che non sono ancora state create.

Se si desidera chiudere lo stream indipendentemente dal fatto che la lettura abbia esito positivo, è necessario inserirla definitivamente.

Non assegnare nulla alle variabili e quindi confrontarle per vedere se qualcosa è accaduto prima; invece strutturare il programma in modo che il percorso in cui si chiude lo stream possa essere raggiunto solo se l'eccezione non viene generata. A parte le variabili utilizzate per iterare per cicli, le variabili non dovrebbero avere bisogno di modificare il valore - tendo a contrassegnare tutto finale a meno che non ci sia l'obbligo di fare altrimenti. Avere dei flag attorno al tuo programma per dirti come sei arrivato al codice attualmente in esecuzione, e quindi cambiare comportamento in base a quei flag, è uno stile procedurale molto (non ancora strutturato) di programmazione.

Come nidificare i blocchi try/catch/finally dipende se si desidera gestire le eccezioni generate dalle diverse fasi in modo diverso.

private static final String questionUrl = "http://stackoverflow.com/questions/3044510/"; 

public static void main (String...args) 
{ 
    try { 
     final URLConnection connection = new URL (args.length > 0 ? args[0] : questionUrl).openConnection(); 

     final BufferedReader br = new BufferedReader (new InputStreamReader (
        connection.getInputStream(), getEncoding (connection))); 

     try { 
      final String response = br.readLine(); 

      System.out.println (response); 
     } catch (IOException e) { 
      // exception handling for reading from reader 
     } finally { 
      // br is final and cannot be null. no need to check 
      br.close(); 
     } 
    } catch (UnsupportedEncodingException uee) { 
     // exception handling for unsupported character encoding 
    } catch (IOException e) { 
     // exception handling for connecting and opening reader 
     // or for closing reader 
    } 
} 

getEncoding esigenze per controllare i risultati della connessione del getContentEncoding() e getContentType() di determinare la codifica della pagina web; il tuo codice utilizza solo la codifica predefinita della piattaforma, che potrebbe essere errata.

Il tuo esempio però è insolito in termini strutturati, poiché è molto procedurale; Normalmente si dovrebbe separare la stampa ed il recupero in un sistema più ampio, e consentire il codice del client per gestire qualsiasi eccezione (o, talvolta, di cattura e creare un'eccezione personalizzata):

public static void main (String...args) 
{ 
    final GetOneLine getOneLine = new GetOneLine(); 

    try { 
     final String value = getOneLine.retrieve (new URL (args.length > 0 ? args[0] : questionUrl)); 
     System.out.println (value); 
    } catch (IOException e) { 
     // exception handling for retrieving one line of text 
    } 
} 

public String retrieve (URL url) throws IOException 
{ 
    final URLConnection connection = url.openConnection(); 
    final InputStream in = connection.getInputStream(); 

    try { 
     final BufferedReader br = new BufferedReader (new InputStreamReader (
        in, getEncoding (connection))); 

     try { 
      return br.readLine(); 
     } finally { 
      br.close(); 
     } 
    } finally { 
     in.close(); 
    } 
} 

Come McDowell ha sottolineato, potrebbe essere necessario chiudere il flusso di input se new InputStreamReader genera.

1

userei comuni apache IO per questo, come altri hanno suggerito, soprattutto IOUtils.toString(InputStream) e IOUtils.closeQuietly(InputStream):

public String readFromUrl(final String url) { 

    InputStream stream = null; // keep this for finally block 

    try { 
     stream = new URL(url).openConnection().getInputStream(); // don't keep unused locals 
     return IOUtils.toString(stream); 
    } catch (final IOException e) { 
     // handle IO errors here (probably not like this) 
     throw new IllegalStateException("Can't read URL " + url, e); 
    } finally { 
     // close the stream here, if it's null, it will be ignored 
     IOUtils.closeQuietly(stream); 
    } 

} 
Problemi correlati