はじめに

2024年のコードレビューの中で個人的に見た、プログラミング言語によらない厄介なコードと対処法、そして伝え方の記事です。

3位 if文ネスト多重層

select文を知らなくて、if文を100個並べて書くとかは見たことがあったのですが、ネストを2桁層重ねる人は初めて見ました。

C#

string today = "5"; // 今日の日付

if (today.Contains("1"))
{
    if (today.Contains("2"))
    {
        if (today.Contains("3"))
        {
            if (today.Contains("4"))
            {
                ...
                Console.WriteLine("success");
                ...
            }
        }
    }
}

しかもコードを動かしても一番深いところまでは行けませんでした。

対処法と伝え方

まずはどのような意図があったのか聞き取りを行います。
次に実際に 一緒に動かしてみて 、最深部までいかないことを確認してもらいます。
そして現状のコードの 問題点 を伝えます。今回の場合だと、以下の通りです。

コードの可読性が低い (他の人が理解しやすくすることが重要)
複雑なif文は、修正する際にバグを生む原因になりやすい
将来的に条件を変更・追加する際に、どこを修正すればいいのか分かりにくい

コード作成者には必要な条件の部分だけ切り出して、以下の改善策提案し、 実際に書いてもらい ました。

ガード節

string today = "5"; // 今日の日付

if (!today.Contains("1")) return;
if (!today.Contains("2")) return;
if (!today.Contains("3")) return;

Console.WriteLine("success");

最後に、コードを書いたら「実際に動かして確認する」よう念押ししました。
コードを書くことに満足して書いたコードを実際に動かさないことが常な人もいます。

2位グローバルな変数i,j,k

グローバル変数名がi, j, kだった時は、一度画面から目を離しました。
グローバル変数を使われると、どこでどんな影響があるか分からず無闇に使ってほしくないのですが、for文で使いがちな変数名i, j, kが使われると、もうコードの動作意図の範囲が分かりません。

対処法と伝え方

グローバル変数の便利さとスコープの重要性を一緒に再確認して、プログラミングでは慣習的にfor文でi, j, kを利用しがちなので、せめて変数名を変えるよう伝えました。
慣習なのでプログラミング経験の浅い初心者が知らなかったとしても仕方がないとは思います。

1位今までの処理は関係ない

長い関数の処理を順に上から辿っていって、やっと最後まで来たと思ったら今までの処理は全く関係なく、Falseを返されていた時は「お前の存在がFalseじゃ」と私の頭の中のおじいさんが言っていました。

対処法と伝え方

コードの確認指示と削除要請を行いました。

おわりに

2024年は色々なコードを見る経験で自分の精神力が鍛えられて勉強になりました。
2025年も自分が成長しつつ、成長させていきたいです。