本記事は、エキサイトホールディングス Advent Calendar 2023の10日目のものになります。
リンクはこちら

レガシーコードとは

時代遅れになってしまった古いソースコード。特に、内容が複雑で、ある開発者独自の仕様で書かれたもの。
コトバンクのデジタル大辞泉から引用

要するに書いてから時が経っており、内容が複雑でわかりにくいコードを全般的に指すのかな...

別の言葉では...

レガシーコードとは、単にテストのないコードである。
レガシーコード改善ガイドから引用

なるほど、テストコードがなければレガシコードだと...

つまり、色々な意味があるんだな...と
今回私が取り上げるのは、テストコードに限定しているものなのではなく、「レガシーコードだと思うコードや良くないコードを取り上げ、それに対してこう直す努力をしていこう！」という話です。
＊読者が意図と違っていたらすみません...タイトルが悪いです...

本事例は、実際に私が似たようなことに遭遇したものを載せていきます。
あくまで"似た"ものなので、全く同じではないです。

レガシコードの例と直す努力

1, 嘘をつくコードやコメント達

いや、コードやコメントが嘘つく訳ない！
そう思いますが、意外と多いです。

例

/*
* メールアドレスを変更する
* 
* @param int $user_id
* @param string $change_to_mail_address
* @param string $change_to_name
*/
public function changeUserMailAdrress(int $user_id, string $user_mail_address, string $change_to_name) {
    ...
}

コメントとメソッド名が、「メールアドレス」を変更することが察するかと思います。
しかし、扱う値を見ると「名前」を変更しそうなものが含まれています。

実は、こちらは、メールアドレスの変更と名前の変更を同時に行うメソッドになっています。
つまり、コメントとメソッド名が嘘をついていることになってしまっています。

【なぜこうなったか】
「名前」の変更は、後からロジックを追加され、追加した実装者が、メソッド名、コメントを修正し忘れたのでしょう。

【何を気をつければよかったか】
コードはもちろん直さないといけないですが、コメント文は直されにくいです。
必ず、周辺コードを見て、コメントで間違ったことが書かれていないか確認しましょう。
特にメソッド内もですが、メソッド外は忘れやすいです。必ず利用箇所や呼び出されている箇所など確認しましょう。

2, ベタ書き

つらつらと長いコードを読み解くのはかなり苦労します。

例

// ユーザー情報を取得する
$Sql = "SELECT * FROM USER WHERE user_id = :user_id";
// $SqlのSQLコードを使う
... ... ...
$result = $Sqlの結果

$user_mail_address = [];
foreach ($result as $row) {
    $user_mail_address = $row['mail_address'];
    $user_rank = $row['user_rank'];
    ...
} 

// ユーザーランクによりメール送信内容を変える
Aランクの処理
... ... ...

Bランクの処理
... ... ...

Cランクの処理
... ... ...

Dランクの処理
... ... ...

これだけの量の処理を同じクラスでベタ書きされているとどこまでが何の処理なのかわからなくなります。

【どうすべきか】
細かくロジックわけしましょう。

ユーザー情報を取得する
取得したユーザー一覧を整理する
ランク別処理（A, B, C, Dそれぞれで分ける）

この時、メソッド単位かクラス単位かは、処理の大きさによります。
例えば、ユーザー情報の取得や取得ユーザーの情報を整理するのは、個人的にはメソッド単位で良さそうです。
ランク別処理は、クラス化して、A, B, C, Dをそのクラス内でメソッド化しても良さそうです！

3, 変数名がわかりにくい&省略を多様する

最初の頃かなり苦労しました。初心者ほどやりがちかつ、プロジェクトコードで出会うと「うっ」ってなってしまうものかと思います。例をいくつか紹介します。

例1

$pcAddr
$mobMailAddr

Addrなんだかわかりますか？
アドレス = Addressの略です。
さらに言えば、ユーザー登録になぜか2種類のメールアドレスを管理しているケースがあり、それによって処理を分けている箇所などがあり、かなり複雑です。

例２

$user_opt_item
$u_k

例1よりわかりにくいのを用意しました。
optは多分optionの略かと思います。
optionのitemもかなり謎な変数名で何が言いたいのかわかりません。(コメントもなければ利用箇所まで遡る必要があります。)
$u_kは、実は、userIdのことです。なんで"k"なのかはわかりません。こういった当時の略語が変数やDBのカラム名などに残ってきます。DBのカラム名に使われると変更箇所も多岐にわたって直すのも少し大変です。

【どうすべきか】
略するのはやめましょう。なるべくそのままの意味で変数名を考えましょう。
また、似たような変数名を多様するとわかりくくなります。本当にその変数が必要なのか考えてみましょう。

4, *の多様

SELECT文などでデータベースから何かしらデータを取得する処理はよくあると思います。
ただ、取得の仕方を間違えると何を取得しているのかわからなくなります。

例

SELECT * FROM USER;

なんてことないUSERテーブルから取得しています。
ただ、こういったSQLをそのままプロジェクトコードに落とすと何が起きるか。
それは、いらないものも必要であるかの様に取得します。

例
USERテーブルには、下記情報が含まれています。
USER_ID: ユーザーID
USER_MAIL_ADDRESS: メールアドレス
USER_NAME: ユーザー名
...etc...

$Sql = "SELECT * FROM USER WHERE user_id = :user_id";
// $SqlのSQLコードを使う
... ... ...
$result = $Sqlの結果

// 以下、メールアドレスを使ってゴニョゴニョする
...

上記の様なコードに資料したとします。
あれ？でも利用しているのは、USER_MAIL_ADDRESSだけの様に見えます。
そうです。必要ないのに取得しています。

今回は、メールアドレスだけを利用しましたが、これが、複数利用されて、コードが複雑になると何が使われているかがパッと見てわからないと何をしているのかわかりにくくなります。

必ず、「＊」で濁さず書き出しましょう。
（SQLの実行速度等にも影響してきます。）

終わりに

今回書き出したのは、全体の20%もないです。
ロジック的に複雑な点や仕様をかなり説明しないと何がまずいのか言いにくい物がたくさんあります。
あくまでよくあるし説明も簡単なものに絞って記述しました。

これが全てだと思わないでいただけるとありがたいですし、対処法として具体的に挙げたわけでもなく「こうした方がいいよね」って言っている程度なので、参考程度になれば幸いです。

余談

技術記事をあまり書いたことがないため下手で申し訳ないです。
徐々に精進していきます。