try/catchの使い方

Question

try/catchの使い方なのですが。

下記はファイルをコピーペーストする関数になります。
処理が正常に行われたかを判断するためにtry/catchを使用したのですが、行数が倍くらいになりました。
処理結果が判断できて、かつ行数の少ない記述を考えています。

下記の様な行数の減らし方は思いつきました。
・部分的にtry/catchを使用する。
・void型にして関数の呼び出し元でtry/catchを使用する。
・この関数に限って言えば、try/catchなしの関数を使用して、下記を追記する。

if(!new File(pathTo).exists()){処理}

2番目が妥当でしょうか。
それとも可読性よりは機能性を優先した方が良いでしょうか。

---

try/catchなし

    //ファイルコピー・貼り付け(try/chatchなし)
    private boolean CopyPaste(String pathFrom, String pathTo) throws IOException {

        //コピー元
        FileInputStream fileIn   = new FileInputStream(pathFrom);

        //コピー先
        FileOutputStream fileOut = new FileOutputStream(pathTo);

        // byte型の配列を宣言
        byte[] buf = new byte[256];
        int len;

        // ファイルの終わりまで読み込む
        while((len = fileIn.read(buf)) != -1){
            fileOut.write(buf,0,len);
        }

        //ファイルに内容を書き込む
        fileOut.flush();

        //ファイルの終了処理
        fileOut.close();
        fileIn.close();

        return true;
    }

---

try/catchあり

    //ファイルコピー・貼り付け(try/catchあり)
    private boolean CopyPaste(String pathFrom, String pathTo) {

        boolean bl = false;

        FileInputStream fileIn   = null;
        FileOutputStream fileOut = null;

        //コピー元
        try {
            fileIn = new FileInputStream(pathFrom);
        } catch (FileNotFoundException e) {
            e.printStackTrace();
            return bl;
        }

        //コピー先
        try {
            fileOut = new FileOutputStream(pathTo);
        } catch (FileNotFoundException e) {
            e.printStackTrace();
            return bl;
        }

        // byte型の配列を宣言
        byte[] buf = new byte[256];
        int len;

        // ファイルの終わりまで読み込む
        try {
            while((len = fileIn.read(buf)) != -1){
                try {
                    fileOut.write(buf,0,len);
                } catch (IOException e) {
                    e.printStackTrace();
                    return bl;
                }
            }
        } catch (IOException e) {
            e.printStackTrace();
            return bl;
        }

        //ファイルに内容を書き込む
        try {
            fileOut.flush();
        } catch (IOException e) {
            e.printStackTrace();
            return bl;
        }

        //ファイルの終了処理
        try {
            fileOut.close();
        } catch (IOException e) {
            e.printStackTrace();
            return bl;
        }
        try {
            fileIn.close();
        } catch (IOException e) {
            e.printStackTrace();
            return bl;
        }

        return true;
    }

Comments

[swordone]

コピー先のファイルがコピー前に存在している必要があるのはいいのでしょうか…。

Answer 1

ご質問のプログラムについていえばtry/catch(詳しく言うとtry-with-resource)は必須と思います。なぜなら資源の解放が必要だからです。throws句をおいたとすると

// メソッド名をキャメルケースに変更
private void copyPaste(String pathFrom, String pathTo) throws IOException {
  try (FileInputStream fileIn = new FileInputStream(...);
       FileOutputStream fileOut = new FileOutputStream(...)) {
    ...なにかする...
  } // catch/finallyなどでcloseを一々書く必要はない。tryから抜けるときに自動的にcloseされる
}

Comments

[KSwordOfHaste]

maisumakunさんがtry/catchの複雑さの軽減について回答されてますので、参考にしただきたいと思います。ただtry-catchを完全に省略するとファイルのクローズ漏れ（資源解放漏れ）となるため、その点のみtry-with-resourceが必要であることに留意していただきたいと思いました。

[uer03108]

有難うございました。
返事が遅れまして申し訳ありません。
リソースを解放することが目的であれば、tryのみの記述で良いという意味でしょうか。
細かな点ですが重要ですね。
大量のファイルを呼び出す場合や、定期的にファイル呼び出しを行う場合でも処理落ちしにくい設計ということでしょうか。

[KSwordOfHaste]

> リソースを解放することが目的であれば、tryのみの記述で良いという意味でしょうか。

そうですね。catch/finallyを必要とするのはアプリケーション固有のなんらかの後処理をしたい場合と考えた方がよい気がします。

一方AutoClosableなオブジェクト(java.io.*の多くのクラスがそうです)の資源漏れを確実に排除するにはtry-with-resourceが常とう手段だと思います。というかtry-with-resource意外の方法でcloseするのはNGと言われるレベルかも知れません。というのは、catch/finallyでcloseを保証するコードは第三者(あるいは自分)が見たときにバグがないかどうかすぐには把握しづらいからです。

> 細かな点ですが重要ですね。

重要であることは同意します。ただ、個人的な意見としては「細かな点」とは考えない方がよいと思っています。

> 大量のファイルを呼び出す場合や、定期的にファイル呼び出しを行う場合でも処理落ちしにくい設計

たとえ１つしかファイルを処理しないようなアプリケーションでも普段からtry-with-resourceを使うことは大切だと思います。そういう習慣をつけておかないといざ「長期間安定して動作するアプリケーションを作ろう」としても、きちんと配慮した設計をやりそこねてしまう危険が高くなるでしょうから。

Answer 2

元のメソッドがthrows IOExceptionとなっている以上は、内部で後始末が必要でもなければtry-catchをせず、呼んだ側に処理を任せるのが正解と考えます（なお、FileNotFoundExceptionもIOExceptionのサブクラスなので、別途宣言する必要はありません）。

throws IOExceptionを外して「失敗すればfalseを返す」メソッドとして実装したい場合も、例外の起きた場所によって挙動を変えるのでなければ、まとめてtry-catchでかまいません。

private boolean someMethod()
{
  try{
    // 本筋の処理
  }catch (IOException e) {
    e.printStackTrace();
    return false;
  }
}

Comments

[uer03108]

＞まとめてtry-catchでかまいません。
勉強になりましたm(_ _)m
まとめて書けるのですね。