はじめに
当たり前だと思うのですが、意外と守られていないことがあります。その1つの例として、条件分岐でのbooleanの扱いを挙げてみます。
~~タイトルには「Javaでは」と書きましたが、おそらくどの言語でも同じです。~~JavaScriptなど他の言語では当てはまらないので取り消します1。
何がダメか
明確にコーディング規約で否定しているケースもありますが2、Javaでは以下のようなコードは書いてはいけません。
if (isAdmin == true) {
// isAdminがtrueのとき
}
if (isAdmin == false) {
// isAdminがfalseのとき
}
理由を3つ書きます。
理由1: 冗長である
上記のコードは以下のように書くことができます。こちらの方がシンプルです。
if (isAdmin) {
// isAdminがtrueのとき
}
if (!isAdmin) {
// isAdminがfalseのとき
}
if〜else〜の場合は三項演算子で書くとシンプルになることもあります。ただし、三項演算子を乱用すると、可読性が落ちる場合があります。詳細は以下の記事を参照してください。
String userType;
if (isAdmin) {
userType = "管理者";
} else {
userType = "一般";
}
// 以下のように三項演算子で書くほうがシンプルになる
String userType = isAdmin ? "管理者" : "一般";
trueの場合は問題ないと思うのですが、falseの場合は!が目立ちにくいケースがあります。
if (!obj.getFoo().isBar()) {
// 処理
}
こういうときは、説明用変数を追加すると良いでしょう。
boolean bar = obj.getFoo().isBar();
if (!bar) {
// 処理
}
理由2: 代入と間違える可能性がある
Javaではif文に書くのはboolean型と決まっているので、if文に代入を書くと通常はコンパイルエラーになります。しかし、boolean型の場合、以下のように書いても、警告が出ずにコンパイルは通ってしまいます。
if (isAdmin = true) {
// isAdminがtrueのとき・・・のはずが常に実行される
}
if (isAdmin = false) {
// isAdminがfalseのとき・・・のはずが常に実行されない
}
// これはコンパイルエラーになる
if (isAdmin = 1) {
}
だからといって以下のようにヨーダ記法3で書くのは本末転倒です。
if (true == isAdmin) {
// isAdminがtrueのとき
}
if (false == isAdmin) {
// isAdminがfalseのとき
}
実際は、IDEやFindBugs/SpotBugs4で警告を出してくれるため、気づかないはずがないのですが、最初からif (isAdmin == true)という書き方をしなければ問題ありません。
理由3: (理論的には)遅くなる
以下の内容はb == trueのときのみで、b == falseや!bを使ったときは影響ありません。詳しくはコメントを参照してください。
これはほとんど余談ですが、理論的には無駄な処理が入ってしまいます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: メソッドから値を戻す
- 0: ローカル変数1(引数
- bar()
- 0: ローカル変数1(引数
b)をスタックに入れる - 1: スタックの値が
0なら、6:に移動 - 4: 定数1(おそらく
1)をスタックに入れる - 5: メソッドから値を戻す
- 6: 定数0(おそらく
0)をスタックに入れる - 7: メソッドから値を戻す
- 0: ローカル変数1(引数
おわりに
「これくらいどっちでもいいじゃないか」という人がいるかもしれませんが、こういうちょっとしたことの積み重ねが、将来大きな差となって表れます。少しずつでも良い習慣をつけていって欲しいです。
追記
反響があったようなので少しずつ補足していきます。
-
flagという変数名は悪い→その通りなので記述を追加しました。- それすら読んでもらえないようなので全部書き換えました。
- 他の言語(JavaScriptなど)では当てはまらない→その通りなので取り消しました。
- 三項演算子は読みにくい→ケースバイケースなので、少し表現を弱めました。
-
JavaScriptの場合: Logical Operators - JavaScript | MDN ↩
-
Javaコーディング規約 – Future Enterprise Coding Standards - フューチャーアーキテクト ↩
-
QBA: 論理式で boolean リテラル値を代入しているメソッド (QBA_QUESTIONABLE_BOOLEAN_ASSIGNMENT) ↩
-
一応計測しましたが差は確認できませんでした。なので「理論上は」です。 ↩
Comments
その通り、明らかに冗長で無駄なのです。
それなのにどうしてこの様なコードが世の中から消えてなくならないかというと、「設計書には『someObject.isEnable()がtrueの時』と書け。そしてそれに忠実に『
if (someObject.isEnable() == true)』とコーディングしろ」という頭痛が痛くなってフランスに渡米しそうなルールがIT業界の一部でいまだに蔓延っているからなのです…（業務的に意味のある変数名を付けようとしたらクラス名をキャメルにしたシステムハンガリアンにしろと言われたりするのもあるあるネタ）
そんな古代のルールはいい加減捨てて欲しいですね
まあ
なんかの理由はあるかな...、よほどでなければ目くじらは立てず、正しく動くように作ってあるほうが優先されるんじゃないかな(というかしそうです>自分)
あと、遅くなるのは、コンパイラが賢ければ最適化される話ではあるかも
Java以外の言語、例えば、Javascriptではif文の中にboolean以外を入れることができます。(Ruby, pythonもですかね)
このような言語において、nullやundefinedなどもif文で制御するというのはよくあります。
そのため == true がないと逆にわかりづらかったといったことは多くありましたね。
今回のこのような話が当てはまるとするならば、Javaなどのように型が決まっている言語でのお話だけだと思います。
(個人的には言語の特性に依存した書き方をすると他言語になれた人が混乱するので、たとえ冗長であれ、== true とわざと書くこともあります)
私もこれ、いくつかの現場で主張しましたが、なぜか受け入れてもらえませんでした。
コメントで触れられている通り、言語による部分はありますし、目くじら立ててレビューで指摘するような事はしませんが。
個人的には、「頭痛が痛い」みたいな文章を読んでいるようで気持ちが悪いですね。
個人的には、
if (hoge == 10)に不自然さを感じないのと同様、if (hoge == true)にも不自然さを感じません。この話のは、1990年代中頃のC FAQの「if ((hoge == fuga) == TRUE)と書くな」という話からの変形だと思っていて、C FAQのそれは確かに冗長だが、
if (hoge == true)は冗長だとは思いません。読みやすいし。コメントありがとうございます。少しずつ回答していきます。
とりあえず、明らかに間違っている箇所については修正・撤回しました。
Swift も当てはまらないのではないかと思います：
JavaScript と違って確かに Bool 以外の判定は文法上不可能なので、確かに
flag == trueは冗長な気もしますが、ただ Swift の場合、!は== false以外にも、Optional の Forced Unwrapping の機能があり、後者は比較的に危険な操作であるがゆえに、なるべく後者の方を目立たせた方が保守しやすい面があります（まあそもそもflagとかいう変数名自体がSwiftのコーディング規約的にアウトではありますのでisXxxとかhasXxxとか使った方がいいですが、ただ別にif isXxx == trueと書いてもそこまで気にはならないですかね）こちらは Swift の文法上不可能であり、間違って
if (flag = true)と書いてしまうとコンパイルエラーになりますので Swift においてはこれは不成立ですこちらは Debug ビルドは把握していませんが、少なくとも Release ビルドでは確実に最終的に同じバイナリーが書き出されますので、こちらも Swift においては不成立です
true / false 以外の値で真偽を表すことができる言語の場合、flag == true のような書き方を強制することはバグの元になります。なので主張されていることは正しいです。
例えばJavaScriptでは undefined, null, 0, "" も「偽」で、偽でない値はすべて「真」になります。多くのライブラリ関数はこれを利用しており、例えば String オブジェクトのメソッド match() は、マッチが成功した場合に配列を返し(これは真と解釈される)、失敗した場合は null (偽と解釈される)を返します。ですので、
if (str.match(re) == true)と書いてしまうと期待した動作とならず、if (str.match(re))と書かなければなりません。他の言語について不注意に言及するのはダメですね(汗)
ドラフトはタイトルに「Java」が付いてなかったのですが、付けておいてよかった。。。
JavaScriptは得意ではないですが、自分は
===で比較するのが好きですね。flagという変数名が悪いのは百も承知ですが、これも本題とは外れるので、
「flagという変数名は避けてください」というコメントでご容赦を。
@hiuchida さんの指摘ですが、
確認したところ、
flag == falseでも!flagでも同じバイトコードでした。まあ「余談」ではあるんですが、詳しくないことを書くものじゃないですね。。。
一旦ここで切ります。
本題。
何で「書いてはいけない」と強い否定表現をしたのかというと、とあるサイトで、意図しない代入
if (isAdmin = true)による問題を避ける方法として、ヨーダ記法if (true == isAdmin)を挙げていたのを見たからです。間違った前提、ここでは
if (isAdmin)でなくif (isAdmin == true)という書き方を前提として、間違った解決方法(ヨーダ記法)を使うのがおかしい、そのためには、間違った前提で考えるのを止めないといけない、そのために強い否定表現を使いました。そこまで分かっててあえて別の表現を使うことは否定しませんが、若い人は型を守る(守破離の「守」)ところから始めないと、後々苦労すると思います。「新人プログラマ応援」というタグを付けたのはそのためです。
あと、C言語のときのクセは、少なくともJavaを使うときには止めたほうがいいと思います。別の記事で書くかもしれませんが、C言語では初期化しないと不定値になるため初期化が推奨されますが(少なくとも1995年頃はそうだった)、Javaは変数を初期化するとむしろ損なケースもあります。なので、他の言語のクセは持ち込まないほうがいいです。
個人的には==trueとは書かないけど、別に書いてあっても気になりませんね。なお、冗長である、以外の理由は違うのでは?
変数名の例がよくないのでなんともいえないのですが、可読性が高い方を選べば良い気がします。
if(option1 == true)
if(isValid)
一律に禁止するレベルの話ではないと思います。
いろんな言語を使っている身としては、下記ルールで統一されていると読みやすいです。
Swiftの回し者じゃないです。普段書いてるのはJavaScript,Java,Python。
あと、このレベルの
「(理論的には)遅くなる」
は気にしない方がいいと思います。
今時の言語だと自分が書いたプログラムが最終的にどうやって実行されるかなんて追いきれないですし、追う必要もないと思います。
こちらはその通りなので修正しました。
以下のように実例もありますが、
「使用頻度が高い」か「メソッドチェーンを多用する」という理由で追加すべきであって、
!が目立たないからという理由はちょっとどうかと思います。そこまでしてJavaで
!を避ける理由が知りたいです。。。タイトルにJavaでは、とされているのであくまでJava限定で話をしますが、Javaの標準API、メジャーなJavaのライブラリ群（apache-commonsやAWSのJavaSDKなど）において、
if (bool変数 == true) の記法を認めているのをほとんど見たことはありません。
（ゼロではないです。例えば、Java8の7000ファイルほどあるソースを全検索したら、過去の遺産的なcom.sunパッケージ以下でいくつかヒットしました。java.パッケージ以下に絞っても3件ありました）
開発者は基本的に、その言語の標準のAPIのソースコード、メジャーなライブラリ、SDKのソースコードと自分たちの作成するソースコード、両者を読みながら開発を進めると思うのです。デバッグで元をたどっていく過程で、自作コードからいつのまにかAPIのコードに上っていくこともあるでしょう。
そんな過程において自作のコード側にのみ、標準APIやメジャーライブラリでは出てこない、無意味で冗長な記法が混ざることは単純に「意図が明確に伝わりにくくなり」「読みにくく」なる要因にしかならないと思います。記法は統一されていた方が望ましく、統一するのならスタンダード側に寄せるのが当たり前じゃないですか？
（うちの開発ではAPIのコード、ライブラリのコードを読んだりしません、というのであれば、好きにしたら良いでしょうが）
全世界の主流のJava開発者が慣れ親しんでいる記法より「こっちの方が自分には読みやすい」とプロジェクトメンバーに主張されても、私なら却下します。
同様の理由で、独自のprefixルールなどの導入にも反対です。APIやライブラリのコードと自作のコードを「異なるルール・記法で書かれたものとして切り替えながら読む」ことは完全に無駄なコストだと思っています。
念のためこの記事を書いた後にいくつか有名プロジェクトのソースコードを読みましたが、
== trueは見つかりませんでした。== falseはElasticsearchで見たくらいです(ちょっとびっくりしましたが、全体的には読みやすいコードです)。必ずしも有名プロジェクトだから品質の高いコードばかりであるとは言えませんが、スタンダードから外れて「こっちの方が自分には読みやすい」と言っていいのは、守破離の「離」の段階に達した人だけだと思います。
C#だとnull許容値型でbool?が可能だから、
if (isAdmin == true){}は普通に使うなぁ。良くあるのは
if(Content?.Items?.Contains(a => a.IsDirectory) == true)みたいなパターン。boolが返ってくる関数でもnull条件演算子を使ってたら返ってくるのはbool?だし、後からnull対策とかもすることもあるから必要なくても
== trueを付けることはある。他の言語でも似たような例は結構あるんじゃないかな？
Javaにnull条件演算子が導入されたら(もうされたりする？)
== trueは氾濫しそう。後はJavaScriptの例が挙がってるがif条件文内がbool型じゃなくても良い言語なら有無は判断しないといけない。
@kurema
C# だと 直接でも
is trueでも== trueでも 同様の IL の出力になるので問題ないやつですね。Let's comment your feelings that are more than good