2

Javaを使い、整数かどうかを判定するメソッドを作成しました

private boolean isInteger(String str) {
    Integer parsedStr;

    try {
        parsedStr= Integer.parseInt(str);
    } catch (NumberFormatException e) {
        return false;
    } catch (NullPointerException e) {
        return false;
    }

    return true;
}

ただこのメソッドはEffective Javaに書かれてる項目57に違反しているようです

例外は、その名が示す通り、例外的条件に対してのみ使用するべきです。通常の制御フローに対しては、決して使用すべきではありません。

そこで項目57の違反を回避するために正規表現を使い仮引数の文字列が整数かどうか判定する方法を思いつきました

private boolean isInteger(String str) {
    Matcher matcher = PATTERN.matcher(str); // PATTERNはどこかで定義するものとする
    return matcher.matches();
}

ただ自分自身正規表現に慣れていないこともあり、正規表現を使うよりもparseintメソッドの例外を捕捉する処理の法が効率と可読性がいいのではないかと感じています

一般的には項目57の違反かもしれませんが、このような例では、デメリットを上回るメリットがあるように感じています

ここで質問なのですが、このような整数を判定する処理で、例外を捕捉するべきではない(項目57に違反しているデメリットが大きい)でしょうか?

1

2 件の回答 2

4

isInteger()メソッドがどういう意味のメソッドなのかです。二つのパターンを考えてみましょう。

  1. Integer.parseInt(String)エラー無く解析可能な文字列ならば、真を返す。でなければ、偽を返す。なお、nullの場合も「エラー無く解析可能な文字列」ではないので、偽を返す。
  2. 整数の十進数表現であれば真を返す。そうで無ければ、偽を返す。ただし、nullの場合はNullPointerExceptionが発生する。

もし、1.であれば、今の実装から大きく変更する必要は無いと思います。なぜなら、意味そのままの実装であるからです。むしろ、無理に正規表現で実装しようとした場合、intの範囲を超えるような整数の前後で区別することが困難です(単純に\A[+-]?0*\d{1,10}\zという正規表現では、"2147483647""2147483648"もマッチしますが、"2147483648"intの範囲を超えているので、Integer.parseInt()はエラーになるため、偽にすべき処理を別途入れる必要があります)。他にもInteger.parseInt()は所謂全角数字(U+FF10'0'~U+FF19'9')も解釈可能ですので、正規表現はより複雑になります。ですので、Integer.parseInt()を例外処理を使うことは、読みやすさや実装しやすさを考えれば、べつに普通かと思います。これを、無理に文字列解析をしようとすると、ほぼ、Integer.parseInt()の実装と同じことをする事になり、無駄が多いかと思います。敢えて直すとすれば、NullPointerExceptioncatchせずに、早期リターンすると良いかもしれません。

private boolean isInteger(String str) {
    if (str == null) return false;

    try {
        Integer.parseInt(str);
    } catch (NumberFormatException e) {
        return false;
    }

    return true;
}

Integer.parseInt()の実装とは関係無く、もっと言えば、int等には関係無く、「整数の十進数表現」であるかどうかを確認したいのであれば、2.のような意味かも知れません。isIntではなくisIntegerとしているのもそういう意図も考えられます。

「整数の十進数表現」はどういうものかです。たとえば、次のような定義が考えられます。

  • U+0030'0'からU+0039'9'の範囲の一つ以上の文字から構成される。
  • 先頭にU+002B'+'またはU+002D'-'を付けても良い。
  • 桁数に制限は無い。
  • 左側にいくつでもU+0030'0'埋めされていてもよい。("42"だけではなく"00000042"としてもよいということ)

このような定義なら、正規表現を使うと、\A[+-]?\d+\zになるので、次のようになるでしょう。

private boolean isInteger(String str) {
    return java.util.regex.Pattern.matches("\\A[+-]?\\d+\\z", str);
}

必ずしも、正規表現を使う必要はありません。正規表現が苦手なら、次のようにしても良いでしょう。

private static boolean isInteger(String str) {
    if (str.isEmpty()) return false;

    var first = str.charAt(0);
    var skip = first == '+' || first == '-' ? 1 : 0;
    return str.chars().skip(skip).allMatch(c -> c >= '0' && c <= '9');
}

もちろん、この2.の意味の場合は、Integer.parseInt()を使っての実装はできないことになります。


Effective Javaも含めて、世の中には「こういうコードの書き方がお勧め」という本や記事がたくさんありますが、無理に従う必要はありません。原則に従うのでは無く、原則の意味が今のコードに当て嵌まるのか、それによって逆に可読性や効率の悪化が起きていないかは検討すべきです。こうすれば正解というのはありません。よりよりものを求める事は大事ですが、求めすぎても、返って効率を落とすだけのこともあるので、要はバランス感覚のようなものが大事なのかも知れません。

4

Effective Javaは読んでませんが、引用されている文章が、他で言われているような例外処理についてのベストプラクティスと同じ趣旨であるのであれば、「例外を投げるメソッドを使ってはならない。別の手段で書き換えるべき」という主張ではありません

isIntergerが例外を投げるのかどうか、その例外がどう使われるのかがポイントです。isIntegerの内部実装がどうなってるかはポイントではありません。

isIntegerが条件を満たさなかったときに例外を投げるメソッドで、それが単に上位メソッドのパラメータチェックに使われるようなものだとしたら、それはバッドプラクティスである、という話です。

この質問に回答するには、ログインする必要があります。

求めていた回答ではありませんか? のタグが付いた他の質問を参照する。