112
57

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

else を禁止してはいけない

Last updated at Posted at 2022-06-07

複雑なプログラムを書いてしまうのを避けるために、if 文の else を避ける制約を設けてプログラミングしてみましょう、というアプローチがあります。これはあくまで、ThoughtWorks アンソロジーに書かれていた、ある種のプログラミング思考のトレーニング方法のひとつにすぎないものです。

しかし、権威だからか、あるいは、あまりコードを書かない人がわかりやすいと感じるからか、プロダクションコードの規約に使おうとする誤った考えがあります。そんなことをすると、コード品質は下がります。

else 禁止制約というのは、「これまで if-else で記述していた多くのコードは、実は、生成するオブジェクトの多態性によって、分岐の前倒しが可能になるものだったのか」という認識を得られれば、すぐに忘れるべきルールです。

ある実在の OSS コードを例に説明します。
https://github.com/xrstf/ip-utils

else なしスタイル

以下は Factory.php ファイルに含まれるコードですが、あらかじめ PHP>=8 で読みやすいように少し書き換えています。else を使っていないから、イコール良いコードだと言えるでしょうか。return のインデントが揃っていません。実行順序を追って理解しないと読めないコードで、循環的複雑度は高めです。内側の if を別のプライベートメソッドにむりやり切り出しても、本質的な複雑度は同じです。

    public static function getExpression(string $expr): ExpressionInterface
    {
		if (!str_contains($expr, '/')) {
			if (!str_contains($expr, '*')) {
				return new Literal($expr);
			}
			return new Pattern($expr);
		}
		return new Subnet($expr);
	}

きちんと else を書く

else ありスタイルに書き換えました。return のインデントは少し揃います。何よりいいのは、これでこのメソッドの実行コードは、どんな条件でも 1 行だけだというのが機械的にわかるようになりました。else には、「ケース A の場合ケース B は無視していい」と、構造を明確に切る役目があります。

    public static function getExpression(string $expr): ExpressionInterface
    {
		if (!str_contains($expr, '/')) {
			if (!str_contains($expr, '*')) {
				return new Literal($expr);
			} else {
                return new Pattern($expr);
            }
        } else {
            return new Subnet($expr);
        }
    }

良い IDE を使えば、書き換えミスの心配なしに、否定を使わない表現に書き換え可能です。

    public static function getExpression(string $expr): ExpressionInterface
    {
		if (str_contains($expr, '/')) {
            return new Subnet($expr);
        } else {
            if (str_contains($expr, '*')) {
                return new Pattern($expr);
            } else {
                return new Literal($expr);
            }
        }
    }

else-if も活用する

else の中にひとつだけ if-else ブロックがネストしている構造は、else-if に書き換え可能です。インデントが浅くなり、循環的複雑度がずいぶん下がりました。このコードは、「生成するのは 3 種類のオブジェクトのうちどれか」を判断するものだということが一目瞭然になりました。

    public static function getExpression(string $expr): ExpressionInterface
    {
		if (str_contains($expr, '/')) {
            return new Subnet($expr);
        } elseif (str_contains($expr, '*')) {
            return new Pattern($expr);
        } else {
            return new Literal($expr);
        }
    }

より適切な意味表現に近づいたことを認識する

ここまでのリファクタリングで、このメソッドは本質的には手続きが必要ないことしかやっていないことが明らかになりました。ひとつの式で表現できます。

        return str_contains($expr, '/') ? new Subnet($expr) : (
            str_contains($expr, '*') ? new Pattern($expr) :
                new Literal($expr)
        );

冗談です。

if 構造をパターンマッチ(優先度あり)だと認識するほうが合っていそうです。三項演算子でも書けるということは、それが確実に可能だという確信を得るステップでした。PHP では少しトリッキーな書き方になりますが、このように、もともとこのメソッドでやりたかったことは循環的複雑度と無縁なことだった、とわかります。

    public static function getExpression(string $expr): ExpressionInterface
    {
        return match(true) {
            str_contains($expr, '/') => new Subnet($expr),
            str_contains($expr, '*') => new Pattern($expr),
            default => new Literal($expr)
        };
    }

もし仮に if-else でしか記述できない状況だったとしても、本質的にこのパターンマッチと同じだと認識できる、直前の else-if を使うかたちの方がいいに決まっています。

訓練で重要なのは、多角的な問題認識がまだ足りない素人に、「ちょっと片方の眼を閉じてやってみなさい。問題のひとつはこういうことです。わかりましたか。では明日はもう片方の目を閉じる練習で教えます」みたいな疑似体験を得てもらうことです。

実戦では両目を開けて戦いましょう。

112
57
13

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
112
57

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?