Poprawny idiom do zarządzania wieloma połączonymi zasobami w bloku Try-with-Resources?

168

Składnia try-with-resources Java 7 (znana również jako blok ARM ( Automatyczne zarządzanie zasobami )) jest przyjemna, krótka i prosta, gdy używasz tylko jednego AutoCloseablezasobu. Nie jestem jednak pewien, jaki jest poprawny idiom, gdy muszę zadeklarować wiele zasobów, które są od siebie zależne, na przykład a FileWriteri a, BufferedWriterktóre to zawijają. Oczywiście to pytanie dotyczy każdego przypadku, gdy niektóre AutoCloseablezasoby są opakowane, a nie tylko te dwie konkretne klasy.

Wymyśliłem trzy następujące alternatywy:

1)

Naiwny idiom, który widziałem, polega na zadeklarowaniu tylko opakowania najwyższego poziomu w zmiennej zarządzanej przez ARM:

static void printToFile1(String text, File file) {
    try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

To jest ładne i krótkie, ale jest zepsute. Ponieważ element bazowy FileWriternie jest zadeklarowany w zmiennej, nigdy nie zostanie zamknięty bezpośrednio w wygenerowanym finallybloku. Zostanie zamknięty tylko closemetodą zawijania BufferedWriter. Problem polega na tym, że jeśli wyjątek zostanie wyrzucony z bwkonstruktora, closeto nie zostanie on wywołany i dlatego element bazowy FileWriter nie zostanie zamknięty .

2)

static void printToFile2(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
            BufferedWriter bw = new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Tutaj zarówno zasób bazowy, jak i zasób opakowujący są zadeklarowane w zmiennych zarządzanych przez ARM, więc oba z pewnością zostaną zamknięte, ale element bazowy fw.close() zostanie wywołany dwukrotnie : nie tylko bezpośrednio, ale także poprzez opakowanie bw.close().

Nie powinno to stanowić problemu dla tych dwóch konkretnych klas, które obie implementują Closeable(co jest podtypem AutoCloseable), których umowa stanowi, że closedozwolone jest wielokrotne wywoływanie :

Zamyka ten strumień i zwalnia wszystkie powiązane z nim zasoby systemowe. Jeśli strumień jest już zamknięty, wywołanie tej metody nie ma żadnego efektu.

Jednak w ogólnym przypadku mogę mieć zasoby, które implementują tylko AutoCloseable(i nie Closeable), co nie gwarantuje, że closemożna to wywołać wiele razy:

Zauważ, że w przeciwieństwie do metody close java.io.Closeable, ta metoda close nie musi być idempotentna. Innymi słowy, wywołanie tej metody close więcej niż raz może mieć widoczny efekt uboczny, w przeciwieństwie do Closeable.close, które jest wymagane, aby nie wywoływać żadnego efektu, jeśli zostanie wywołane więcej niż raz. Jednak implementujących ten interfejs zdecydowanie zachęca się, aby ich bliskie metody były idempotentne.

3)

static void printToFile3(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        BufferedWriter bw = new BufferedWriter(fw);
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Ta wersja powinna być teoretycznie poprawna, ponieważ tylko fwreprezentuje prawdziwy zasób, który należy wyczyścić. Samo bwnie zawiera żadnego zasobu, a jedynie deleguje go do fw, więc powinno wystarczyć tylko zamknięcie bazowego fw.

Z drugiej strony składnia jest nieco nieregularna, a także Eclipse generuje ostrzeżenie, które uważam za fałszywy alarm, ale nadal jest to ostrzeżenie, z którym trzeba sobie poradzić:

Wyciek zasobów: „bw” nigdy nie jest zamykany


Więc jakie podejście wybrać? A może przegapiłem jakiś inny idiom, który jest właściwy ?

Natix
źródło
4
Oczywiście, jeśli bazowy konstruktor FileWriter zgłasza wyjątek, nie zostaje nawet otwarty i wszystko jest w porządku. Pierwszy przykład dotyczy tego, co się stanie, jeśli zostanie utworzony FileWriter, ale konstruktor BufferedWriter zgłasza wyjątek.
Natix
6
Warto zauważyć, że BufferedWriter nie zgłosi wyjątku. Czy jest jakiś przykład, który możesz wymyślić, gdzie to pytanie nie jest czysto akademickie.
Peter Lawrey
10
@PeterLawrey Tak, masz rację, że konstruktor BufferedWriter w tym scenariuszu najprawdopodobniej nie zgłosi wyjątku, ale jak już wspomniałem, to pytanie dotyczy wszelkich zasobów w stylu dekoratora. Ale na przykład public BufferedWriter(Writer out, int sz)może rzucić IllegalArgumentException. Mogę również rozszerzyć BufferedWriter o klasę, która wyrzuci coś ze swojego konstruktora lub utworzy dowolne niestandardowe opakowanie, którego potrzebuję.
Natix
5
BufferedWriterKonstruktor łatwo wyrzucić wyjątek. OutOfMemoryErrorjest prawdopodobnie najczęstszym, ponieważ przydziela sporą część pamięci dla bufora (chociaż może wskazywać, że chcesz ponownie uruchomić cały proces). / Trzeba flushTWOJEJ BufferedWriterjeśli nie blisko i chcesz zachować zawartość (generalnie tylko i uzasadnienie bez wyjątku). FileWriterprzejmuje wszystko, co jest „domyślnym” kodowaniem pliku - lepiej jest to wyraźnie określić.
Tom Hawtin - tackline
10
@Natix Chciałbym, żeby wszystkie pytania w SO były tak dobrze zbadane i jasne, jak to jest. Chciałbym móc głosować w tym głosowaniu 100 razy.
Geek,

Odpowiedzi:

75

Oto moje podejście do alternatyw:

1)

try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
    bw.write(text);
}

Dla mnie najlepszą rzeczą, która pojawiła się w Javie z tradycyjnego C ++ 15 lat temu, było to, że możesz zaufać swojemu programowi. Nawet jeśli coś jest w błocie i idzie źle, co często się zdarza, chcę, aby reszta kodu dotyczyła najlepszego zachowania i zapachu róż. Rzeczywiście, BufferedWritermoże to rzucić tutaj wyjątek. Na przykład brak pamięci nie byłby niczym niezwykłym. W przypadku innych dekoratorów, czy wiesz, która z java.ioklas opakowania rzuca sprawdzony wyjątek od swoich konstruktorów? Ja nie. Zrozumiałość kodu nie jest zbyt dobra, jeśli polegasz na tego rodzaju niejasnej wiedzy.

Jest też „zniszczenie”. Jeśli wystąpi stan błędu, prawdopodobnie nie chcesz wyrzucać śmieci do pliku, który wymaga usunięcia (kod do tego nie jest wyświetlany). Chociaż oczywiście usunięcie pliku jest również kolejną interesującą operacją do wykonania jako obsługa błędów.

Ogólnie rzecz biorąc, chcesz, aby finallybloki były jak najkrótsze i niezawodne. Dodawanie kolorów nie pomaga w osiągnięciu tego celu. W wielu wydaniach niektóre z klas buforujących w JDK miały błąd polegający na tym, że wyjątek od flushwewnątrz closespowodowany closena dekorowanym obiekcie nie był wywoływany. Chociaż zostało to naprawione od jakiegoś czasu, spodziewaj się tego po innych implementacjach.

2)

try (
    FileWriter fw = new FileWriter(file);
    BufferedWriter bw = new BufferedWriter(fw)
) {
    bw.write(text);
}

Nadal opróżniamy niejawny blok końcowy (teraz z powtórzeniem close- to pogarsza się, gdy dodajesz więcej dekoratorów), ale konstrukcja jest bezpieczna i musimy ostatecznie zablokować niejawne, więc nawet niepowodzenie flushnie zapobiega uwolnieniu zasobów.

3)

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
}

Tu jest błąd. Powinien być:

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
    bw.flush();
}

Niektóre źle wdrożone dekoratory są w rzeczywistości zasobami i będą wymagały niezawodnego zamknięcia. Ponadto niektóre strumienie mogą wymagać zamknięcia w określony sposób (być może używają kompresji i muszą pisać bity, aby zakończyć, i nie mogą po prostu opróżnić wszystkiego.

Werdykt

Chociaż 3 jest technicznie lepszym rozwiązaniem, ze względu na rozwój oprogramowania 2 jest lepszym wyborem. Jednak próba z zasobem jest nadal niewystarczająca i należy trzymać się idiomu Execute Around , który powinien mieć jaśniejszą składnię z zamknięciami w Javie SE 8.

Tom Hawtin - haczyk
źródło
4
W wersji 3, skąd wiesz, że bw nie wymaga wywołania jego bliskości? A nawet jeśli możesz być pewien, czy tak jest, czy nie jest to również „niejasna wiedza”, o której wspomniałeś w wersji 1?
TimK
3
Ze względu na rozwój oprogramowania 2 są lepszym wyborem ” Czy możesz wyjaśnić to stwierdzenie bardziej szczegółowo?
Duncan Jones
8
Czy możesz podać przykład idiomu „Execute Around with closures”
Markus,
2
Czy możesz wyjaśnić, na czym polega „jaśniejsza składnia z zamknięciami w Javie SE 8”?
petertc
1
Przykład „Execute Around idiom” jest tutaj: stackoverflow.com/a/342016/258772
mrts
20

Pierwszy styl to ten, który sugeruje Oracle . BufferedWriternie zgłasza sprawdzonych wyjątków, więc jeśli zostanie zgłoszony jakikolwiek wyjątek, program nie powinien się z niego odzyskać, przez co odzyskiwanie zasobów jest głównie dyskusyjne.

Głównie dlatego, że mogło się to zdarzyć w wątku, gdy wątek umierał, ale program nadal działał - powiedzmy, wystąpiła tymczasowa awaria pamięci, która nie była wystarczająco długa, aby poważnie zakłócić resztę programu. Jest to jednak raczej wątpliwy przypadek i jeśli zdarza się to wystarczająco często, aby spowodować wyciek zasobów, próba użycia zasobów jest najmniejszym z twoich problemów.

Daniel C. Sobral
źródło
2
Jest to również zalecane podejście w 3. edycji Effective Java.
shmosel
5

Opcja 4

Zmień zasoby, aby były zamykalne, a nie AutoClosable, jeśli możesz. Fakt, że konstruktory można połączyć w łańcuch, oznacza, że ​​nie jest niczym niezwykłym dwukrotne zamknięcie zasobu. (Było to prawdą również przed ARM.) Więcej na ten temat poniżej.

Opcja 5

Nie używaj ARM i kodu bardzo ostrożnie, aby upewnić się, że close () nie zostanie wywołana dwukrotnie!

Opcja 6

Nie używaj ARM i wykonuj w końcu wywołania close () w trybie try / catch.

Dlaczego nie sądzę, że ten problem jest unikalny dla ARM

We wszystkich tych przykładach wywołania last close () powinny znajdować się w bloku catch. Pominięto ze względu na czytelność.

Nic dobrego, ponieważ fw można zamknąć dwukrotnie. (co jest dobre w przypadku FileWriter, ale nie w twoim hipotetycznym przykładzie):

FileWriter fw = null;
BufferedWriter bw = null;
try {
  fw = new FileWriter(file);
  bw = new BufferedWriter(fw);
  bw.write(text);
} finally {
  if ( fw != null ) fw.close();
  if ( bw != null ) bw.close();
}

Nic dobrego, ponieważ fw nie jest zamknięty, jeśli wyjątek podczas konstruowania BufferedWriter. (znowu nie może się zdarzyć, ale w twoim hipotetycznym przykładzie):

FileWriter fw = null;
BufferedWriter bw = null;
try {
  fw = new FileWriter(file);
  bw = new BufferedWriter(fw);
  bw.write(text);
} finally {
  if ( bw != null ) bw.close();
}
Jeanne Boyarsky
źródło
3

Chciałem tylko oprzeć się na sugestii Jeanne Boyarsky, aby nie używać ARM, ale upewnić się, że FileWriter jest zawsze zamykany dokładnie raz. Nie myśl, że są tu jakieś problemy ...

FileWriter fw = null;
BufferedWriter bw = null;
try {
    fw = new FileWriter(file);
    bw = new BufferedWriter(fw);
    bw.write(text);
} finally {
    if (bw != null) bw.close();
    else if (fw != null) fw.close();
}

Wydaje mi się, że ponieważ ARM to tylko cukier syntaktyczny, nie zawsze możemy go użyć do zastąpienia ostatecznie bloków. Tak jak nie zawsze możemy użyć pętli for-each, aby zrobić coś, co jest możliwe za pomocą iteratorów.

AmyGamy
źródło
5
Jeśli zarówno twój, jak tryi finallybloki rzucają wyjątki, ta konstrukcja utraci pierwszy (i potencjalnie bardziej przydatny).
rxg
3

Zgadzając się z wcześniejszymi komentarzami: najprościej jest (2) użyć Closeablezasobów i zadeklarować je w kolejności w klauzuli try-with-resources. Jeśli masz tylko AutoCloseable, możesz umieścić je w innej (zagnieżdżonej) klasie, która sprawdza closetylko raz wywołaną (wzorzec elewacji), np. Przez posiadanie private bool isClosed;. W praktyce nawet Oracle tylko (1) łączy w łańcuch konstruktory i nie obsługuje poprawnie wyjątków w części łańcucha.

Alternatywnie możesz ręcznie utworzyć połączony zasób, używając statycznej metody fabryki; to hermetyzuje łańcuch i obsługuje czyszczenie, jeśli zawiedzie częściowo:

static BufferedWriter createBufferedWriterFromFile(File file)
  throws IOException {
  // If constructor throws an exception, no resource acquired, so no release required.
  FileWriter fileWriter = new FileWriter(file);
  try {
    return new BufferedWriter(fileWriter);  
  } catch (IOException newBufferedWriterException) {
    try {
      fileWriter.close();
    } catch (IOException closeException) {
      // Exceptions in cleanup code are secondary to exceptions in primary code (body of try),
      // as in try-with-resources.
      newBufferedWriterException.addSuppressed(closeException);
    }
    throw newBufferedWriterException;
  }
}

Następnie możesz użyć go jako pojedynczego zasobu w klauzuli try-with-resources:

try (BufferedWriter writer = createBufferedWriterFromFile(file)) {
  // Work with writer.
}

Złożoność wynika z obsługi wielu wyjątków; w przeciwnym razie jest to po prostu „zamknij zasoby, które zdobyłeś do tej pory”. Powszechną praktyką wydaje się być najpierw zainicjowanie zmiennej, która przechowuje obiekt, który przechowuje zasób null(tutaj fileWriter), a następnie uwzględnienie zerowego sprawdzenia w czyszczeniu, ale wydaje się to niepotrzebne: jeśli konstruktor zawiedzie, nie ma nic do wyczyszczenia, więc możemy po prostu pozwolić na propagację tego wyjątku, co trochę upraszcza kod.

Prawdopodobnie możesz to zrobić ogólnie:

static <T extends AutoCloseable, U extends AutoCloseable, V>
    T createChainedResource(V v) throws Exception {
  // If constructor throws an exception, no resource acquired, so no release required.
  U u = new U(v);
  try {
    return new T(u);  
  } catch (Exception newTException) {
    try {
      u.close();
    } catch (Exception closeException) {
      // Exceptions in cleanup code are secondary to exceptions in primary code (body of try),
      // as in try-with-resources.
      newTException.addSuppressed(closeException);
    }
    throw newTException;
  }
}

Podobnie możesz połączyć trzy zasoby itp.

Na marginesie matematycznym możesz nawet trzykrotnie łączyć w łańcuch, łącząc dwa zasoby naraz, i byłoby to asocjacyjne, co oznacza, że ​​otrzymałeś ten sam obiekt w przypadku sukcesu (ponieważ konstruktory są asocjacyjne) i te same wyjątki, jeśli wystąpi awaria w dowolnym konstruktorze. Zakładając, że dodałeś S do powyższego łańcucha (więc zaczynasz od V i kończysz na S , stosując kolejno U , T i S ), otrzymasz to samo, jeśli najpierw połączysz S i T , a następnie U , odpowiadające (ST) U lub jeśli najpierw połączyłeś T i U , toS , odpowiadające S (TU) . Jednak byłoby jaśniejsze, gdybyśmy po prostu napisali jawny potrójny łańcuch w jednej funkcji fabryki.

Nils von Barth
źródło
Czy poprawnie zbieram, że nadal trzeba używać try-with-resource, jak w try (BufferedWriter writer = <BufferedWriter, FileWriter>createChainedResource(file)) { /* work with writer */ }?
ErikE
@ErikE Tak, nadal musiałbyś używać try-with-resources, ale musiałbyś użyć tylko jednej funkcji dla połączonego zasobu: funkcja fabryki hermetyzuje łańcuch. Dodałem przykład użycia; dzięki!
Nils von Barth
2

Ponieważ zasoby są zagnieżdżone, klauzule try-with powinny również wyglądać następująco:

try (FileWriter fw=new FileWriter(file)) {
    try (BufferedWriter bw=new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
} catch (IOException ex) {
    // handle ex
}
zatruć
źródło
5
Jest to bardzo podobne do mojego drugiego przykładu. Jeśli nie wystąpi żaden wyjątek, element FileWriter closezostanie wywołany dwukrotnie.
Natix
0

Powiedziałbym, że nie używaj ARM i kontynuuj z Closeable. Użyj metody takiej jak,

public void close(Closeable... closeables) {
    for (Closeable closeable: closeables) {
       try {
           closeable.close();
         } catch (IOException e) {
           // you can't much for this
          }
    }

}

Powinieneś także rozważyć wywołanie close of, BufferedWriterponieważ nie jest to tylko delegowanie zamknięcia FileWriter, ale także porządkowanie flushBuffer.

sakthisundar
źródło
0

Moim rozwiązaniem jest refaktoryzacja „metody wyodrębniania” w następujący sposób:

static AutoCloseable writeFileWriter(FileWriter fw, String txt) throws IOException{
    final BufferedWriter bw  = new BufferedWriter(fw);
    bw.write(txt);
    return new AutoCloseable(){

        @Override
        public void close() throws IOException {
            bw.flush();
        }

    };
}

printToFile można zapisać

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        AutoCloseable w = writeFileWriter(fw, text);
        w.close();
    } catch (Exception ex) {
        // handle ex
    }
}

lub

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
        AutoCloseable w = writeFileWriter(fw, text)){

    } catch (Exception ex) {
        // handle ex
    }
}

Projektantom bibliotek klasowych zasugeruję rozszerzenie AutoClosable interfejsu o dodatkową metodę, aby pominąć zamknięcie. W takim przypadku możemy ręcznie kontrolować zachowanie zamknięcia.

Dla projektantów języków lekcja jest taka, że ​​dodanie nowej funkcji może oznaczać dodanie wielu innych. W tym przypadku Javy, oczywiście, funkcja ARM będzie działać lepiej z mechanizmem przenoszenia własności zasobów.

AKTUALIZACJA

Oryginalnie powyższy kod wymaga, @SuppressWarningponieważ BufferedWriterwnętrze funkcji wymagaclose() .

Jak sugeruje komentarz, jeśli flush()chcemy zostać wywołani przed zamknięciem modułu zapisującego, musimy to zrobić przed wszelkimi return(niejawnymi lub jawnymi) instrukcjami wewnątrz bloku try. Myślę, że obecnie nie ma sposobu, aby upewnić się, że dzwoniący robi to, więc musi to zostać udokumentowane writeFileWriter.

ZAKTUALIZUJ PONOWNIE

Powyższa aktualizacja sprawia, że @SuppressWarning zbędna, ponieważ wymaga funkcji zwrócenia zasobu wywołującemu, więc sama nie musi być zamykana. Niestety, to ciągnie nas z powrotem do początku sytuacji: ostrzeżenie jest teraz przenoszone z powrotem na stronę dzwoniącego.

Aby poprawnie rozwiązać ten problem, potrzebujemy spersonalizowanego ustawienia, AutoClosableże za każdym razem, gdy się zamyka, podkreślenie BufferedWriterzostanie flush()zredagowane. Właściwie to pokazuje nam inny sposób obejścia ostrzeżenia, ponieważ BufferWriternigdy nie jest zamknięty w żaden sposób.

Earth Engine
źródło
Ostrzeżenie ma swoje znaczenie: czy możemy być tutaj pewni, że bwfaktycznie wypisze dane? W końcu jest buforowany, więc musi tylko czasami zapisywać na dysku (gdy bufor jest pełny i / lub włączony flush()i close()metody). Myślę, że flush()należy nazwać metodę. Ale i tak nie ma potrzeby używania buforowanego programu zapisującego, gdy piszemy go natychmiast w jednej partii. A jeśli nie naprawisz kodu, możesz skończyć z danymi zapisanymi w pliku w złej kolejności lub nawet w ogóle nie zapisanymi w pliku.
Petr Janeček,
Jeśli flush()wymagane jest wywołanie, nastąpi to zawsze, zanim dzwoniący zdecyduje się zamknąć FileWriter. Więc powinno to nastąpić printToFiletuż przed jakimkolwiek returns w bloku try. Nie powinno to być częścią, writeFileWritera zatem ostrzeżenie nie dotyczy niczego wewnątrz tej funkcji, ale wywołującego tę funkcję. Jeśli mamy adnotację @LiftWarningToCaller("wanrningXXX"), pomoże to w tej sprawie i podobnej.
Earth Engine