現場で学ぶプログラミング【スクールだと学べない!?】 | エンジニアが投資でセミリタイヤを目指すブログ

ども、エンジニア大家です。

数年前からソースコードのレビューをする側の人間になりましたが、

現場に入ったばかりかつ、経験があまりない方の場合に同じような事で躓いていることが多い印象を持ちました。

プログラミング教室や独学だと気が付かない内容なのかな？と思ったので、今回は記事にしてみました。

現場で学ぶプログラミング

新人さんにコードレビューで指摘した例で説明したいと思います。

差分に余計なものが含まれている

プログラミング以前の話になっちゃいますが、最近だとgitを利用してソースレビューをすることが多いですが、
明らかにタスク範囲外の差分が出ていました。

それを指摘したところ、

新人さん

あー、〇〇行目〜△△行目までだけレビューして他の行の差分は無視してください

的なことを言われたことがありました。

原因は不明ですが、おそらく開発の初期で段階でマージ先とは関係ないブランチから開発ブランチを作成したと思われます。

そして、差分の内容はマージ先のブランチ（この場合developブランチ）にマージされてしまうことをどうやら知らないようです。

そのことを教えてあげて「修正対象だけ差分が出るようにやり直してください」と伝えると、

新人さん

じゃー、最初からやり直します（●｀ε´●）

と不満気なお答えを頂きました。

in_arrayは「型」で判定すべき

いきなり細かい内容になりますが、PHPでin_arrayを使った判定の場合、

第三引数にtrueを入れないと挙動がおかしくなることがあります。

詳細は以下を見てください。

参考 in_arrayを使うときは黙って第三引数を付けることQiita

簡単に説明すると、比較する際に「型」まで見る必要があるので常に第三引数にはtrueを入れとけということです。

現場のルールだと第三引数はtrueを入れる事になっていた

ちょっと脱線しますが、私が居た現場のルールだと第三引数はtrueを入れる事になっていました。

☓in_array(1, $array)
○in_array(1, $array, true)

ですが、レビュワーが複数いまして、「第三引数を入れなくても問題がない場合」はそのルールを無視してレビューを通してる場合もあり、結果として記述が統一されてませんでした。

という状況でしたが私がルール通りに指摘すると、、

新人さん

なんだよ、他の人だったら問題なくレビューが通るんだけど！？

的な文句を言われてしまいました。

SQLのSELECT文をループ内で複数発行している

これもケースバイケースではありますが、

ループの中で毎回SELECTするよりもループの手前でSELECTを一度行うと効率が良い場合があります。

foreach($rows as $row){
	$member = $db->query("SELECT * FROM members WHERE id = " . $row['member_id']);
	if($member['mail_address'] !== null){
		...
	}
}

ではなくて、

$member_ids = array_column($rows, 'member_id');
$members = $db->query("SELECT * FROM members WHERE id IN (" .  implode(',', $member_ids) . ")"); //ループ手前で取得
$members = array_column($members, null, 'id');
foreach($rows as $row){
	$member = $members[$row['member_id']]; //取得済みデータを使う
	if($member['mail_address'] !== null){
		...
	}
}

のようにすれば会員テーブルへの問い合わせは一回で済みます。

SQLが複数回ではなく一回の方がDB負荷が少なくて済みます。