Bool型の扱い- 良いコード書くきっかけを与えたい。7[C#リファクタリングサンプル]

#Bool型の扱い
Bool型の変数名や関数名はtrueとなることが想定できるような名前にするという規約を見かける。例えば、is+名詞句、形容詞（あるいは形容詞のみ）や過去分詞などを使用するなどである。
ここでは、trueが想定しにくい例を挙げて、この規約に当てはめることを考える。
ちなみに、isをつけるかどうかについては好みの問題だと思う。

##サンプルコード

        public void Main()
        {
            bool findFlg = true;
            bool blnFind = true;
            bool finded = true;
            bool result = false;
            bool notFound = false;

            // NG 変数名からフラグの意味が読み取れない。
            if (findFlg == true)
            {
            }

            // NG 変数名からフラグの意味が読み取れない。
            if (blnFind == true)
            {
            }

            // △ 過去分詞はfound。意味は分かる。
            if (finded)
            {
            }

            // NG 変数名からフラグの意味が読み取れない。
            if (!result)
            {
            }

            // NG 否定をtrueにしているせいで、２重否定になっている。
            if (!notFound)
            {
            }

            int resultType = 1;
            bool success;
            // NG 無駄なif文
            if (resultType == 1)
            {
                success = true;
            }
            else
            {
                success = false;
            }

            bool beAbleToDo = true;
            // △ 規約に違反していないが長すぎる。Canで良い。
            if (beAbleToDo)
            {
            }
        }

##リファクタリング後のコード

     public void Main()
        {
            bool found = true;
            // 過去分詞を使用することで、見つけたらtrueであることが分かる。
            if (found)
            {
            }

            // マジックナンバーは定数にするべきであるが、サンプルではこのままにしておく。
            int resultType = 1;
            // successのままでも意味が通じるが、形容詞にする。
            bool successful;
            successful = (resultType == 1);

            // be able to ではなくcanを使用すれば簡潔になる。
            bool canDo = true;
            if (canDo)
            {
            }
        }

##チェック処理関数名 2017/3/17 追記
bool値を戻り値とするチェック処理の関数については、...Checkと命名する人が非常に多い。これはBool型の規約違反になることに気づいてほしい。
例として入力チェックを行い、値が不正の場合、処理を抜けるサンプルコードをあげる。

        void Main(string inputValue)
        {
            // NG trueを返すのがどんなときか読み取りずらい。またfalseで比較しているのもNG。
            if (InputCheck(inputValue) == false)
            {
                return;
            }

            // △ ２重否定ではないのでOKではあるものの、elseや２重否定が今後発生する可能性がある。
            if (IsInvalid(inputValue))
            {
            }

            // OK 主語がないが値妥当性チェックであることが分かる
            if (!IsValid(inputValue))
            {
                return;
            }

            // OK 主語があり、値妥当性チェックであることが分かる
            if (!IsValueValid(inputValue))
            {
                return;
            }

        }

入力チェック=>InputCheckという名前は発想としては分かるが、bool型の戻り値について説明できていない。bool型の変数だろうと関数だろうと命名規約は同じなので、注意してほしい。
ちなみに、IsInvalidは無効な値をtrueにしており、サンプルコード上は問題なく見える。elseをつけたり否定をつけたりすると可読性が落ちるので、お勧めはしない。

##まとめ
規約に従えば、bool型の変数名の場合、ほとんど悩まずにコーディングできるし読みやすい。一方、サンプルコードで△としたものは規約を守ろうとしているし、意味は通じるので、コードレビューではOKにするかもしれない。

また、bool型を戻り値とする関数名の場合も変数と同じ命名規約に従うことには注意してほしい。特に...Checkは何度指摘しても出てくるので、書かないように気をつけてほしい。

規約は単純であるもの、難しいという意見を現場でもらったことがある。おそらくVB6時代から脱却できていないプログラマにとってはNGで書いているようなコードが読みやすく、今さら変えることはできないのだろう。

前の記事(安易なNullチェック)

次の記事(関数の引数が多すぎる)

目次