Javaではif (flag == true)というコードを書いてはいけない

はじめに

当たり前だと思うのですが、意外と守られていないことがあります。その1つの例として、条件分岐でのbooleanの扱いを挙げてみます。

タイトルには「Javaでは」と書きましたが、おそらくどの言語でも同じです。

何がダメか

明確にコーディング規約で否定しているケースもありますが1、Javaでは以下のようなコードは書いてはいけません

if (flag == true) {
  // flagがtrueのとき
}

if (flag == false) {
  // flagがfalseのとき
}

理由を3つ書きます。

理由1: 冗長である

上記のコードは以下のように書くことができます。こちらの方がシンプルです。

if (flag) {
  // flagがtrueのとき
}

if (!flag) {
  // flagがfalseのとき
}

if〜else〜の場合は三項演算子で書くとシンプルになります2

int a;
if (flag) {
  a = 1;
} else {
  a = 2;
}

// 以下のように三項演算子で書くほうがよい
int a = flag ? 1 : 2;

trueの場合は問題ないと思うのですが、falseの場合は!が目立ちにくいケースがあります。

if (!obj.getFoo().getBar()) {
  // 処理
}

こういうときは、説明用変数を追加すると良いでしょう。

boolean bar = obj.getFoo().getBar();
if (!bar) {
  // 処理
}

理由2: 代入と間違える可能性がある

Javaではif文に書くのはboolean型と決まっているので、if文に代入を書くと通常はコンパイルエラーになります。boolean型の場合、以下のように書いても、警告が出ずにコンパイルは通ってしまいます。

if (flag = true) {
  // flagがtrueのとき・・・のはずが常に実行される
}

if (flag = false) {
  // flagがfalseのとき・・・のはずが常に実行されない
}

// これはコンパイルエラーになる
if (flag = 1) {
}

だからといって以下のようにヨーダ記法3で書くのは本末転倒です。

if (true == flag) {
  // flagがtrueのとき
}

if (false == flag) {
  // flagがfalseのとき
}

実際は、IDEやFindBugs/SpotBugs4で警告を出してくれるため、気づかないはずがないのですが、最初からif (flag == true)という書き方をしなければ問題ありません。

理由3: (理論的には)遅くなる

これはほとんど余談ですが、理論的には無駄な処理が入ってしまいます5。例えば、以下のコードをコンパイルして、javap -cで逆アセンブルします。

public class Main {
    public int foo(boolean b) {
        if (b == true) {
            return 1;
        } else {
            return 0;
        }
    }

    public int bar(boolean b) {
        if (b) {
            return 1;
        } else {
            return 0;
        }
    }
}

すると、以下のような出力が得られます(関係ない出力は除いています)。

  public int foo(boolean);
    Code:
       0: iload_1
       1: iconst_1
       2: if_icmpne     7
       5: iconst_1
       6: ireturn
       7: iconst_0
       8: ireturn

  public int bar(boolean);
    Code:
       0: iload_1
       1: ifeq          6
       4: iconst_1
       5: ireturn
       6: iconst_0
       7: ireturn

これを読み解くと以下のようになります6。長々と書いていますが、要は命令が1つ無駄に多くなってしまいます。

なぜなら、if (b)は「bが真なら」を表しているのに対し、if (b == true)は、「bがtrueと比較した結果が真なら」と回りくどい表現になっているからです。

  • foo()
    • 0: ローカル変数1(引数b)をスタックに入れる
    • 1: 定数1(おそらくtrue)をスタックに入れる
    • 2: スタックの値(0:と1:)を比較し、違ってたら7:に移動
    • 5: 定数1(おそらく1)をスタックに入れる
    • 6: メソッドから値を戻す
    • 7: 定数0(おそらく0)をスタックに入れる
    • 8: メソッドから値を戻す
  • bar()
    • 0: ローカル変数1(引数b)をスタックに入れる
    • 1: スタックの値が0なら、6:に移動
    • 4: 定数1(おそらく1)をスタックに入れる
    • 5: メソッドから値を戻す
    • 6: 定数0(おそらく0)をスタックに入れる
    • 7: メソッドから値を戻す

おわりに

「これくらいどっちでもいいじゃないか」という人がいるかもしれませんが、こういうちょっとしたことの積み重ねが、将来大きな差となって表れます。少しずつでも良い習慣をつけていって欲しいです。


  1. Javaコーディング規約 – Future Enterprise Coding Standards - フューチャーアーキテクト 

  2. 三項演算子を乱用すると可読性が落ちる可能性はありますが、ここではその話はしません。 

  3. ヨーダ記法 - Wikipediaなど 

  4. QBA: 論理式で boolean リテラル値を代入しているメソッド (QBA_QUESTIONABLE_BOOLEAN_ASSIGNMENT) 

  5. 一応計測しましたが差は確認できませんでした。なので「理論上は」です。 

  6. The Java® Virtual Machine Specificationから推測。 

60contribution

その通り、明らかに冗長で無駄なのです。
それなのにどうしてこの様なコードが世の中から消えてなくならないかというと、「設計書には『someObject.isEnable()がtrueの時』と書け。そしてそれに忠実に『if (someObject.isEnable() == true)』とコーディングしろ」という頭痛が痛くなってフランスに渡米しそうなルールがIT業界の一部でいまだに蔓延っているからなのです…
(業務的に意味のある変数名を付けようとしたらクラス名をキャメルにしたシステムハンガリアンにしろと言われたりするのもあるあるネタ)

そんな古代のルールはいい加減捨てて欲しいですね:put_litter_in_its_place:

135contribution

すみません。最初に謝っておきます。この文章の趣旨は理解します。

しかし、命令が1つ多いと言うのはtrue(1)と比較しているためであって、false(0)と比較すると変わらなくなります。

        if (b == false) {
            return 0;
        } else {
            return 1;
        }

       0: iload_1
       1: ifne          6
       4: iconst_0
       5: ireturn
       6: iconst_1
       7: ireturn

これは0から10までループするのと、10から0までループするのと、比較することと似ています。

308contribution

まあ

  • Cの慣習をひきずった場合(Cはあくまで整数だったので、うっかり動作があったりフラグとかビット演算の関係とか)
  • 複合だとフラグの真/偽だけでand/orを書くとわかりにくくなる、そこから単品でのa==true/falseをやってしまう

なんかの理由はあるかな...、よほどでなければ目くじらは立てず、正しく動くように作ってあるほうが優先されるんじゃないかな(というかしそうです>自分)

あと、遅くなるのは、コンパイラが賢ければ最適化される話ではあるかも

Sign up for free and join this conversation.
Sign Up
If you already have a Qiita account log in.