【新人プログラマ向け】開発とソースコードレビューのコツ #PHP

はじめに

この4月から新しくプログラマになった方が大勢いると思います。
ようこそこちら側へ！
私は7年システムエンジニアをやっている者です。
新人プログラマに向けて開発とレビューのコツを伝えられたらいいなと思ってこの記事を書きました。
私が実際に作業する時に意識していることなので100%の正解ではないですが、刺さる部分だけ持って帰ってください！

開発のコツ

コードを書き始める前に

開発する時のフローやルーティンを構築すると良いです。
私の場合はコードを書き始める前のフローを大切にしています。

その開発案件の要件を把握する
- どういう点で困っていて、何をするとゴールなのか
- 言われたことそのまま開発してもゴールじゃない可能性があります
要件を把握したらその周辺の機能やコードについてひたすら調査します
- どういう機能なのか
- どういう構造でできているものなのか
- 新人だと早く着手しなきゃ！と焦っていきなりコードを書きがちですが、調査の時間を多くとった方が結果的に質が高く早く終ることが多いです
周辺調査を終えて開発方針を固めたら先輩や有識者にチェックしてもらう
- まだコードは書き始めません
- 調査した内容が合っているか、開発方針が合っているかをこの段階でチェックしてもらいましょう
- そうすることで出戻りが少なく開発がスムーズに進みます
この時点でようやくコードを書き始めます

実際にコードを書く際に

まずコードの全体像が掴めるように概形を作成しコミットしてしまうことをオススメします。
例えば以下のようなコードに処理を追加する場合、まずは概形を以下のように書くと良いです。（exec_sampleに_exec処理を追加する）
TODOコメント等で何をやるかもこの時点で書いてしまうと、後から「何すればいいんだっけ？」とならないので良いです。

sample.php

<?php
class Sample {
    /**
     * 何らかの実行メソッド
     */
    public function exec_sample(){
        // 何を実行しているのかよく分からないがとりあえず長い処理

        // TODO: ここに処理を差し込む
        $result = $this->_exec();
        // TODO: デバッグ文を後で消す
        var_dump($result);

        // 何を実行しているのかよく分からないがとりあえず長い処理
    }

    /**
     * TODO: doc書く
     *
     * @return string $result
     */
    private function _exec(){
        // TODO: 必要な変数の確保、必要に応じて引数で渡す
        // TODO: $resultは初期値空文字で良いか？
        $result = '';
        // TODO: Aの条件を実装、必要に応じて判定関数化
        if(true){
            // TODO: Aのケースの場合の処理
            $result = 'Aのケースです';
        } elseif(true) {
            // TODO: Bのケースの場合の処理
            $result = 'Bのケースです';
        } elseif(true) {
            // TODO: 他にケースあるか？C以降を検討する
        } else {
            // TODO: どこにも該当しなかった場合の処理
            // 例外を投げるか？
            // catchする必要があるか？親でキャッチしてくれるか？
        }

        return $result;
    }
}

$sample = new Sample();
$sample->exec_sample();

また、処理の共通化等に関してもまずは愚直に書いてから「あ、ここ共通化できそう」と思って後からやるのをオススメします。
慣れない状態で先に共通化を意識して書いていくと肝心のロジックが破綻したりするので。

概形を作る
ロジックを作る
共通化や関数化をする
インデントや使わなかった変数を消すなどのリファクタをし整形する

意識して欲しいことは「まず要件を満たして動くコードを書くことが最優先」ということです。
仕事としてプログラムを書いている以上、正しく意図した通りに動くコードに価値が生まれます。
極端な話、インデントがぐちゃぐちゃになっていようが、未使用の変数が残っていようがその機能を使うユーザーには関係がありません。
もちろんその後の保守性や全体のコードの品質に関わってくるのでキレイにこしたことはないですが、開発時の優先順位は考えた方が良いです。

ソースコードレビューのコツ

少し慣れてきたらソースコードレビューをやる機会もあると思います。
レビューに関しても自分の中でフローを構築すると良いです。

その開発案件の要件を把握する
要件を把握したらプルリクをチェックする
- 書いているコードの意味や意図が分からなければ聞く
その周辺の機能やコードについて調査する
- 類似の機能やコードはないか

レビューする際にオススメなのは手元にコードを持ってきて、改修内容を見つつそのコードにコメントを入れていくことです。
例えば以下のような感じ。

monster_hunterレビュー前.php

<?php
$weapons = ['片手剣','双剣','大剣','太刀','ランス','ガンランス','ハンマー','笛','スラッシュアックス','チャージアックス','操虫棍','ライトボウガン','ヘビィボウガン','弓'];
function make_seed()
{
    list($usec, $sec) = explode(' ', microtime());
    return $sec + $usec * 1000000;
}
srand(make_seed());
$randval = rand(0,13);
var_dump($weapons[$randval]);

monster_hunterレビュー後.php

<?php
// クラス化した方が良さそう
// 手元のスクリプトだし別にそこまでしなくても良いか？

// 武器種の定義
// 武器種の増減があった場合にはここの配列を操作すればよいだけになっている
// 各武器種を定数化した方が今後拡張しやすいか？
$weapons = ['片手剣','双剣','大剣','太刀','ランス','ガンランス','ハンマー','笛','スラッシュアックス','チャージアックス','操虫棍','ライトボウガン','ヘビィボウガン','弓'];

// 乱数のシード値を現在時刻から算出している
// これはphp公式リファレンスに載っているもの
// @see https://www.php.net/manual/ja/function.srand.php
// TODO: docコメント追加してもらう
function make_seed()
{
    // microtimeで現在時刻を取得
    // msec secの形式の文字列を返す
    // 例: 0.05883800 1618473243
    // それを半角スペースで分割してusecとsecに入れている
    // usec = 0.05883800
    // sec  = 1618473243
    list($usec, $sec) = explode(' ', microtime());
    // var_dump($sec + $usec * 1000000);
    // 例: 1618597943のような数値となる
    return $sec + $usec * 1000000;
}
// 乱数生成器を先程のシード値で初期化
srand(make_seed());
// 乱数を0~13で生成する
// TODO: これは武器種数によって変わるので$weaponsのsizeの方が良い、後で指摘する
$randval = rand(0,13);

// 全武器種の中からランダムに1つの武器を選ぶ処理
// 今日何の武器を使うか迷った時に回す想定
// var_dumpでなくprintの方が良さそう
var_dump($weapons[$randval]);

コードに対してコメント文を書いていくと気付いたことがすぐメモできたり、あとで確認する時に漏れが少なくなるのでオススメです。
また実際に動かしつつこの変数にはどういう値が入っていて、どういう使い方をしているか、みたいなメモを残していくとコードリーディングが捗り理解が早くなります。
ここから実際にプルリク上で指摘することを決めてコメントします。
指摘する際は温度感を表すために[MUST]や[SHOULD]などを使うと良いでしょう。

ソースコードレビューのレベル

あくまで自己流ですが、自分の中でレビューに段階（レベル）を設けています。
レビューをする際に「どのレベルまでレビューするか」
またはレビューをしてもらってる際に「どのレベルの指摘なのか」
を考えると自分のコードは今どのレベルの品質になっているかの目安になって良いです。

レベル1: インデントなどのスタイル、誤字、未使用変数
レベル2: ロジックが破綻していないか、意図した通りに動いているか
レベル3: 要件を満たしているか、影響する範囲は正しいか
レベル4: 処理負荷が高くないか、重くないか
レベル5: 設計、この位置にこの処理があるのはキモいなど

レベルの高いレビューをクリアすればするほど良いコードになっていると言えます。
私は案件によってレビューのレベルを変えています。
例えば急ぎ目の案件で来週にはリリースしないといけないような案件ではレベル3までしか見ません。
今後も長く使っていく機能、プロダクトなコアな機能、リファクタ案件などはレベル5まで見ます。
新人の方はまずはレベル3を目指してください。

レベル1に関して、静的解析ツールなど自身で気付ける形にしておくのが良いです。
レベル2に関して、テストコードを書く、網羅テストするなどで担保しておくのが良いです。
レベル3に関して、コードだけでは判断つかない部分だったりするので先輩や有識者を頼ると良いでしょう。
レベル4に関して、大体重いのはループ内でDBアクセスしているものです。
100回select文を実行し、1000回update文を実行している、ようなコードは要注意です。
このあたりは経験がものをいうところが大きいので経験をたくさん積んでください。
N+1クエリ、計算量などがキーワードとなります。
レベル5に関して、これは設計を学ぶ必要があります。
社内に設計に詳しい人がいればその人を頼るといいでしょう。
MVCモデル、クリーンアーキテクチャ、デザインパターンなどがキーワードとなります。

まとめ

慣れないことが多く戸惑うことが多いでしょうが、刺さる方法・やりやすい方法を模索して1人前のエンジニア・プログラマになってください！