テクノロジー

一年前の自分のコードを見てみる

マイナビエンジニアブログ編集部

はじめに

去年の10月に現場に配属されてから、はや1年間が経っていました。(また一つ歳をとってしまいました、、)
1年たった記念に、去年の配属直後に書いていたコードを見返してみました。
気になったところと、改善点をまとめてみます。

まとめてみた

(1) 無駄な改行など

<tr>
       <th style="width: 10rem;">ID</th>
       <td style="width: 20rem;">{{$sampleArray[0]->id}}</td>

</tr>

  • 良くないところ
    全体的に無駄な改行やスペースなどが目立ちます。
    また、var_dump(javascriptで言うところのconsole.log的な)の消し忘れもちらちら。

  • 修正点、対策
    commitやプルリク作成時にしっかり差分をチェックすれば大方防げるかなと思います。
    またformatterの利用も有効だと思います(改行とかは消えないかも？)

(2) 命名が適当

$sample_item = [1, 2, 3];

  • 良くないところ
    配列なのに変数名からそれが分からない

  • 修正点、対策
    配列の場合は複数形にしたり、sample_arrayとかにして、配列であることが分かるようにする。
    配列の変数名に限らず、いろんな変数や関数の命名が雑でした。
    リーダブルコードとかでも散々書かれていることですが、他の人が読んでわかりやすいように、変数や関数の命名には気を配っていきたいです。
    ちなみに、変数名の後ろにくる複数形のsとか_arrayをサフィックスっていうらしいです。

(3) 肥大した関数

$sample_array = [1, 2, 3];
$sample_value1 = sampleFunc();
$sample_value2 = 0;
foreach($sample_array as sample) {
    $sample_value2 += sample * $sample_value1
}
以下何十行、何百行と続く、、

  • 良くないところ
    適切な粒度で関数の切り出しが行われておらず、大きくて見通しの悪いコードとなっている

  • 修正点、対策
    全体として見通しの良いコードとなるように、適切な粒度で関数の切り分けをおこなっていく。
    適切な粒度の頃合いが難しいですが、明らかに肥大しすぎてた関数が多く見受けられます。
    前は関数を切り分けるのは主に処理の共通化のためだと考えていたのですが、例え一箇所でしか使われない関数でも、関心を分離して処理の大まかな流れを掴みやすくするためにも関数を切り分けていくべきだと思っています。

(4) PHPDoc

/**
* サンプル
* @param $sample_param1
* @param $sample_param2
*/
public function sampleFunc($sample_param1, $sample_param2)
{

  • 良くないところ
    PHPDocが適当

  • 修正点、対策
    動的型付け言語のPHPですが、PHPDocというフォーマットでコメントを書くことによって、クラスやメソッドの引数や返り値の型などを示すことができます。
    あくまでコメントなのでこの型を守らなくてもエラーなどは起こりませんが、適切に記述することによってエディタの補完が働いたり、他の開発者が見たときにコードの理解が容易になります。
    (2)の変数名の命名と共通しますが、他の人にとって読みやすいコードという意識が足りなく、他の関数でもなんか書いてるから適当に書いとくかーって感じでやってました💦
    PHPやPythonなどの動的型付け言語に後付けで型に関する情報を追記する際は、それをどこまでやるかはプロジェクト次第だと思います。
    開発のスピード感とのトレードオフになってしまい難しい問題かもしれませんが、PHPDocをちょっと丁寧に書くくらいならそこまで手間じゃないと思うので、今後はしっかり書いていきたいなと思っています。

(5) gitの使い方が雑

sampleCommit1: 一通り完成
sampleCommit2: バグ修正

  • 良くないところ
    commitの粒度が大きすぎる(そこそこ規模の大きなプルリクなのに、一回のcommitにまとめてる)、コミットメッセージが適当、不必要なcommitが混ざっているなど

  • 修正点、対策
    作業の途中ではcommitせずに、完成した段階で全ての変更を一つのcommitにまとめていました。
    ある程度規模の大きいプルリクの場合、適切な粒度でcommitして行った方が良いのかなと思います。
    また、commitのメッセージについても、change、addなどの接頭語を使ったりしてそのcommitがどういった変更なのかが分かりやすいようにできると良いと思います。

(6) パフォーマンスに対する意識(N+1問題などなど)

$prefectures = getPrefectures();
foreach($prefectures as $prefecture) {
    $cities = getCities($prefecture->prefId);
    $prefecture->cities = $cities
}

  • 良くないところ
    上の例のようにfor文の中で都度sql叩くような処理をした結果パフォーマンスが悪くなるのをN+1問題と言います。
    とりあえず動けば良いと考えて、こういったパフォーマンスに対する意識が低いコードが散見されます。

  • 修正点、対策
    sql頑張って書いてワンクエリにするか、hasManyとかのリレーションを宣言してwithとかでとってくるかすればN+1問題解決できます。
    実装時はあまり違いが分からなくても、レコード数の増加によって後から致命的な問題になってくることが多々あるので、N+1問題とかは解決しておいた方が良さそうです。
    場合によってはさらなるチューニングが必要になってくることもあると思いますが、開発速度や可読性とのトレードオフになってくることもあるので、難しいところですね、、、

(7)ほとんどすべてのdb操作の記述をクエリビルダで行っている

$query = sampleTable1::query()
　->leftjoin('sampleTable2', 'sample_column', '=', sampleTable2.sample_column')

  • 良くないところ
    ほとんどすべてのdb操作をクエリビルダで行っていた

  • 修正点、対策
    Laravelではdbの操作を行うにあたって、主にsqlの記述に近いクエリビルダで書く方法とO/RマッパーであるEloquentを使う方法があります。
    なんとなく理解しやすかったのと既存のコードがクエリビルダで書かれているものが多かったのでクエリビルダをつかいがちでした。
    どちらもメリットデメリットあるので、一概にEloquentを使えば良いという訳でもないと思いますが、Eloquentを使った方がすっきりとした記述で済みそうな箇所が結構ありました。
    Eloquentに限らず、Laravelのようなフレームワークには知らないだけでかなり色々な機能が備わっています。
    とりあえず何かしらの方法である実装ができるようになると、その方法で他の同様の実装も済ませてしまいがちですが、もっと便利な方法、推奨されている方法がないか常日頃から意識して調査していきたいですね、、！

(8)場当たり的な対応で凌ごうとする

$sample = sampleFunction1()

// 何故か1秒待たないといけない
sleep(1)

  • 良くないところ
    上の例はちょっと極端ですが、何か実装で詰まった時に問題の根本を調査・理解しようとせずに、既存の実装の変更を最小にして場当たり的な対応のコードを追加していく形で対応しています。

  • 修正点、対策
    一番の解決策は、分からないところを他のメンバーに聞くことだと思います。
    大抵こういう場当たり的な対応は後々問題になります、、
    後々対応すると実装内容の理解から始める必要があり余計に時間がかかってしまうので、最初に実装して詰まった段階で詳しそうな方に聞くのが一番です！
    また、レビューの段階でこういったことを質問して結果大幅な変更が必要になることがわかると実装者も辛いですし、レビュワーもなんか申し訳なくなるので、出来るだけ早い段階で聞いた方が良いし、もっといえば聞ける関係を普段から作れてると良いのかなと思います！

感想

全体的な傾向として、知識と意識が足りなかったです。
そもそもどのようなコードが良いのかが分からなかったですし、N+1問題っていう名前すら知らなかったです。

また良い書き方を知っているところにしても、可読性やパフォーマンスの重要度があまり肌感として持っていないが故に、目の前の仕事を早く終わらせることに夢中になって妥協をしてしまうことが多かったです。

もちろんまだまだ勉強中の身ですが、一年前よりは知識の面でも感覚の面でもマシなコードを書けてるなと思いました。

願わくは、来年の今頃に今年の自分のコードを見て、たくさん改善点を見つけられればと思います。

※本記事は2023年12月時点の内容です。

テクノロジーの記事一覧
タグ一覧
TOPへ戻る

関連記事