- PR -

java.io.BufferedWriter クラスの close メソッドのデザインについての疑問

«前のページへ 1|2|3 次のページへ»
投票結果総投票数：19
親も close すべき 13票 68.42%
親は放っておくべき 6票 31.58%
  • 投票は恣意的に行われます。統計的な調査と異なり、投票データの正確性や標本の代表性は保証されません。
  • 投票結果の正当性や公平性について、＠ITは一切保証も関与もいたしません。
投稿者投稿内容
unibon
ぬし
会議室デビュー日: 2002/08/22
投稿数: 1532
お住まい・勤務地: 美人谷　　　　　　　　良回答(20pt)
投稿日時: 2007-01-14 22:02
引用:

kumaさんの書き込み (2007-01-14 21:11) より:
次のような時、どのようにしたいとしているのでしょうか？
コード:
class A {
  void createHoge() {
    String fileName = "hoge";
    Writer writer = this.getWriter(fileName);
    writer.close();
  }
  Writer getWriter(String fileName) {
    OutputStream os = new FileOutputStream(fileName);
    Writer w = new OutputStreamWriter(os);
    BufferedWriter bw = new BufferedWriter(w);
    return bw;
  }
}


逆質問になりますが、このコードの writer.close(); の部分で os のインスタンスが IOException を投げた場合、どうやって os を close するのでしょうか？私の最初の質問で挙げた例のように、ファイルがオープンされたままになってしまいます。すなわち os の finalize が動くことを期待して、それまでずっと待つことになってしまいます。
そもそも、このコードだと、BufferedWriter クラスの close メソッドが投票の選択肢のいずれの仕様であるかに関わらず、親の os を参照する手段が失われています。

--
unibon {B73D0144-CD2A-11DA-8E06-0050DA15BC86}
unibon
ぬし
会議室デビュー日: 2002/08/22
投稿数: 1532
お住まい・勤務地: 美人谷　　　　　　　　良回答(20pt)
投稿日時: 2007-01-14 22:14
引用:

kumaさんの書き込み (2007-01-14 21:11) より:
次のような時、どのようにしたいとしているのでしょうか？
コード:
class A {
  void createHoge() {
    String fileName = "hoge";
    Writer writer = this.getWriter(fileName);
    writer.close();
  }
  Writer getWriter(String fileName) {
    OutputStream os = new FileOutputStream(fileName);
    Writer w = new OutputStreamWriter(os);
    BufferedWriter bw = new BufferedWriter(w);
    return bw;
  }
}

実際に最外殻のラッパーを使用する側（この場合#createHoge）
がその内部構成について考慮しなければならないのでしょうか？

いわゆる「異常系」まできちんと処理しようとすれば、内部構成(ここでは変数 os が指すインスタンス)も考慮しないといけないと思います。

ちなみにこれは、おおまかには java.io.FileWriter クラスと同じ構造だと思います。FileWriter の javadoc を読むと「簡易クラス」だという但し書きはありますが、異常系も「簡易」に扱っている、ということになるのでしょうか。
kuma
大ベテラン
会議室デビュー日: 2004/02/25
投稿数: 110
投稿日時: 2007-01-14 22:59
下は必要と思われる部分だけ抜き出したBufferedWriterです。
コード:
public class BufferedWriter extends Writer {
    private Writer out;
    public BufferedWriter(Writer out) {
	this(out, defaultCharBufferSize);
    }
    public BufferedWriter(Writer out, int sz) {
	super(out);
	if (sz <= 0)
	    throw new IllegalArgumentException("Buffer size <= 0");
	this.out = out;
	cb = new char[sz];
	nChars = sz;
	nextChar = 0;

	lineSeparator =	(String) java.security.AccessController.doPrivileged(
               new sun.security.action.GetPropertyAction("line.separator"));
    }
    public void close() throws IOException {
	synchronized (lock) {
	    if (out == null)
		return;
	    flushBuffer();
	    out.close();
	    out = null;
	    cb = null;
	}
    }
}

BufferedWriter#closeでExceptionが発生しうる最後がout.close()です。
(ラップしたWriterのclose処理です)
ここでExceptionが発生したのであればBufferedWriterも実際closeはされていませんし
BufferedWriterのoutもラップしたWriterへの参照が残っています。
unibonさんの
「親の os を参照する手段が失われています」
これがどのようなことを指しているのかが、
まだ理解できていません。
Tdnr_Sym
ぬし
会議室デビュー日: 2005/09/13
投稿数: 464
お住まい・勤務地: 明石・神戸
投稿日時: 2007-01-15 00:12
こんばんは。

BufferedWriterのcloseに限らず、Javaにも限らず、
一般的なプログラミングの作法として…

ファイルやＤＢ接続などのリソースのクローズに失敗してしまったら、
あとは素直に「後始末をあきらめる」だけのような気がしますが？

少なくとも私は、「後始末エラー」や「ログ出力エラー」の
異常系は記述しません。「どうしようもないね」とあきらめてます。

＃　私、間違ってます？？
unibon
ぬし
会議室デビュー日: 2002/08/22
投稿数: 1532
お住まい・勤務地: 美人谷　　　　　　　　良回答(20pt)
投稿日時: 2007-01-15 00:13
引用:

kumaさんの書き込み (2007-01-14 22:59) より:
下は必要と思われる部分だけ抜き出したBufferedWriterです。

引用:

kumaさんの書き込み (2007-01-14 22:59) より:
public void close() throws IOException {
synchronized (lock) {
if (out == null)
return;
flushBuffer();
out.close();
out = null;
cb = null;
}
}

私は、現在のところ、この BufferedWriter の close の実装がおかしいと思っています。
(public ではないメソッドである) flushBuffer で IOException が起こりうるのですが、ここで IOException が起こっても起こらなくても out.close(); は実行しなければいけないと思います。
そもそも close の中で close と flush の両方をしている(順序はもちろん flush の後に close)、というようにメソッドが複合的な処理をしていることがまずいのではないでしょうか。flush をむりやり隠蔽しようとして、そのツケが回ってきているように感じます。BufferedWriter はその名のとおりバッファー処理を担当しているわけですから、無理に隠蔽せずに、close の前に flush は必ず呼ばないといけない、そして flush を呼ばずに close を呼んだら (RuntimeException である) IllegalStateException あたりを throw してしまえば良いと思います。
こうすることで close と flush の役割が明確になります。呼び出し側で close したいときにはその前に flush を呼び、たとえばディスクフルで IOException が投げられたら、その時であっても close を呼び直すことができ、その中では flush はやりませんから確実にリソースを解放できます。

もしこういう仕様になっていれば、エラーハンドリングはつぎのように簡素に書けます。
コード:
package foo;

import java.io.BufferedWriter;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;

public class Abc {
    
    public static void main(String[] args) throws FileNotFoundException {
        OutputStream os = new FileOutputStream("c:\\abc.txt");
        Writer w = new OutputStreamWriter(os);
        BufferedWriter bw = null;
        try {
            bw = new BufferedWriter(w, 0x1000);
            for (int i = 0; i < 10; i++) {
                bw.write("愛 = " + i);
                bw.newLine();
            }
            bw.flush();
            bw.close();
            bw = null;
        } catch (IOException ex) {
            // エラー発生のメッセージ出力等は別途必要。
            if (bw != null) {
                try {
                    bw.close(); // bw だけ気にすれば良い。
                } catch (IOException ex2) {
                    throw new RuntimeException(ex2);
                }
            }
        }
    }
}


ちなみに、もし、従来の仕様のままだとつぎのように書かざるを得ません。
コード:
package foo;

import java.io.BufferedWriter;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;

public class Abc {
    
    public static void main(String[] args) throws FileNotFoundException {
        OutputStream os = new FileOutputStream("c:\\abc.txt");
        Writer w = new OutputStreamWriter(os);
        BufferedWriter bw = null;
        try {
            bw = new BufferedWriter(w, 0x1000);
            for (int i = 0; i < 10; i++) {
                bw.write("愛 = " + i);
                bw.newLine();
            }
            bw.close();
        } catch (IOException ex) {
            // エラー発生のメッセージ出力等は別途必要。
            try {
                os.close(); // bw は使えないので、os か w を操作せざるを得ない。
            } catch (IOException ex2) {
                throw new RuntimeException(ex2);
            }
        }
    }
}
unibon
ぬし
会議室デビュー日: 2002/08/22
投稿数: 1532
お住まい・勤務地: 美人谷　　　　　　　　良回答(20pt)
投稿日時: 2007-01-15 00:27
引用:

Tdnr_Symさんの書き込み (2007-01-15 00:12) より:
BufferedWriterのcloseに限らず、Javaにも限らず、
一般的なプログラミングの作法として…

ファイルやＤＢ接続などのリソースのクローズに失敗してしまったら、
あとは素直に「後始末をあきらめる」だけのような気がしますが？

少なくとも私は、「後始末エラー」や「ログ出力エラー」の
異常系は記述しません。「どうしようもないね」とあきらめてます。

たとえば、アプリケーションのメニューからファイル出力操作を選択して実行し、ファイル abc.txt をディスクに書いている途中に、ディスクフルになったとします。その時に"異常系"処理内でファイルをクローズしていないと、そのファイル(とディスクを空けるためにその他のファイルも一緒に)を削除してもう一度、同じファイル名のファイルで出力しようとしてもそれができません。(ファイルの削除も、同名のファイルのオープンもできないわけですから。)

ガーベッジコレクションが動いて、FileOutputStream の finalize が動いてファイルがクローズされるのは何秒後になるか分かりません。ユーザーはそれまでじっと待つのでしょうか？
極端な例とも言えるかもしれませんが、しかし、これぐらいには対処すべきではないでしょうか。少なくとも、Java においてガーベッジコレクション任せになる(finalize に期待する)のは、正しいコードではないと思います。
unibon
ぬし
会議室デビュー日: 2002/08/22
投稿数: 1532
お住まい・勤務地: 美人谷　　　　　　　　良回答(20pt)
投稿日時: 2007-01-15 00:59
引用:

unibonさんの書き込み (2007-01-15 00:13) より:
もしこういう仕様になっていれば、エラーハンドリングはつぎのように簡素に書けます。

引用:

unibonさんの書き込み (2007-01-15 00:13) より:
ちなみに、もし、従来の仕様のままだとつぎのように書かざるを得ません。

余談ですが、"異常系"の処理として catch の中に close を書いてしまいましたが、本来は finally の中に書くべきことだったかもしれません。質問の内容とは直接関係しませんが。
かつのり
ぬし
会議室デビュー日: 2004/03/18
投稿数: 2015
お住まい・勤務地: 札幌
投稿日時: 2007-01-15 01:23
ちょっと余談ですが、私はストリーム系の後始末はBufferedWriterに限らず、
どんなデコレータパターンになっているかわからないと仮定して、
すべてのものを後始末するようにしています。

例えば、こんな感じのコードを用意します。
コード:
public class StreamUtils{
    public static void close(IOException ex, Closable... closables) throws IOException{
        for(Closable c : closables){
            try{
                if(c != null){
                    c.close();
                }
            }catch(IOException e){
                if(ex == null){
                    ex = e;
                }
            }
        }

        if(ex != null){
            throw new IOException(ex);
        }
    }
}

引数の全てのストリームのクローズを保障するメソッドです。
で、利用コード
コード:
InputStream fin = new FileInputStream("hoge");
BufferedReader reader = new BufferedReader(fin);
OutputStream fout = new FileOutputStream("piyo");
BufferedWriter writer = new BufferedWriter(fout);
IOException ex = null;

try{
    処理....
}catch(IOException e){
    ex = e;
}finally{
    StreamUtils.close(ex,fin,fout,writer,reader);
}

これなら比較的すっきりと、例外処理とクローズができます。
«前のページへ 1|2|3 次のページへ»

スキルアップ／キャリアアップ（JOB@IT）