新人エンジニアが約半年で学んだコーディングの品質

こんにちは、新人エンジニアのsatokです。

新卒採用されて約半年が経ちました。
私は学生時代にプログラミングを授業で学び、業務でも役に立てよう！と意気込んでいました。
そんな私がここまでに学んだ「学生時代に身に着けた技術と業務で要求された技術のギャップ」についてまとめましたので
新人エンジニアの皆様は特に、よろしければご一読ください。

1.この記事の目的

この記事では、新人エンジニアである私が、現場に配属されてから受けたコーディングに関する指摘をまとめ、
「品質の良いコードとは？」に対する1つの回答を提示します。
新人エンジニアの皆様の何か助けになればと思います。

2. ダメ出しリスト

早速ですが、私がこれまでに受けてきたコーディングに対する指摘を列挙していきます。
お見苦しいものにはなりますが、どうか寛大なお心でご覧ください。
なお、一部Playwrightにおけるコードが入っておりますので、こちらの記事をご覧いただくとよりわかりやすい場合がございます。

①タブサイズ(スペース数)

私は、入社前はタブサイズ4でずっと作業していたため、入社後もそのままタブサイズを4で作業していました。
しかし、配属された先で、「タブサイズ2のスペースに直してくれ」と言われたため、不思議に思いながらも受け入れました。

そこで私は初めてスタイルガイド(コーディング規約)の存在を知りました。
スタイルガイドとは、タブサイズから細かなメソッドの描き方まで、「強制はしないが推奨はするよ」というレベルで先人達がまとめたものです。
(コーディング規約、と表現すると、現場単位で制定されたルールを指すこともあるようです。)
現場のエンジニアそれぞれがルールを守ることで、美しく、メンテナンス性の高いコードが保たれているわけです。

②1行を長くし過ぎない

高校や大学でプログラミングを授業として学んだ方には共感していただけるかもしれませんが、
コーディング技術を学ぶ一環で、「いかに美しくまとめるか」を考えることがあると思います。

例えば(c言語において)

if(a==b){
  処理A;
}
else{
  処理B;
}

と書けるところを

a==b ? 処理A : 処理処理B;

のような条件演算子を使うようなことですね。
スタイリッシュにはなりますが、これを拗らせると、やたらと1行が長く、可読性が悪いコードが完成します。

また、上の例では故意で1行を長くしていますが、
私の場合は、そのコードの性質上、どうしても1行が長くならざるを得ない状況に陥りました。

例えば以下のようなコードがありました。(こちらの言語はTypeScriptです。)

await expect.soft(page.getByRole('cell', { name: '<とんでもなく長い名前>' }).nth(3), 'No.' + testNumber + ' <とんでもなく長い名前>ボタンエラー').toBeVisible();

このコードでは最初と最後の部分から想像できますが、とあるボタンが表示されているかのチェックをするだけです。
しかし、そのボタンの名前がとにかく長いせいでたった1つの命令が凄い長さになっています。

ここでの問題を考えてみましょう。
1行がとても長くなって困るのは誰か、それは後から読む人です。
1行が長すぎると、この行ではどんな処理が行われているのか、それをパッと読むことが難しくなってしまいます。
しかし、今回の行は削れる部分があまりありません。
(一応名前を別の変数に入れれば短くはできますが、これと同じ形のテストが10個以上あるため得策ではありませんでした。)
ではどうすれば良いか。

問題の本質は、”パッと見て何の処理を行っているかわからない”部分なので、そこを解決すれば良いのです。
今回であれば、重要なのはexpectの部分と.toBeVisible()の部分なので、上のコードはこのように変形できます。

await expect.soft(page.getByRole('cell', { name: '<とんでもなく長い名前>' }).nth(3), 'No.' + testNumber + ' 加<とんでもなく長い名前>ボタンエラー')
    .toBeVisible();

変更した部分は、末尾にあった.toBeVisible()の部分を次の行に持ってきただけです。
これにより、パッと見で何かが表示されているかのチェックをしている、ということが伝わりやすくなりました。
勘違いしていただきたくないのは、ただ改行をたくさん入れれば良いということではなく、
何を伝えたいかをはっきりさせるというところが重要だということです。
つまり、1行が長すぎることによって、相手に伝えたいことが伝わりにくくなることが問題なので、その問題を解決する方法が重要であるということです。

③マジックナンバー

新人エンジニアあるあるだと勝手に思っていますが、マジックナンバーはやめましょう。
マジックナンバーとは、ソースコード(主になんらかの処理)中に直接書き込まれている数字のことです。
よくある例を以下に見せます。

for(int i=0; i<10; i++){なんらかの処理}

c言語風に書きましたが、要するに10回ループしながら何か処理するコードです。
ここでの”10″(場合によっては”0″も)がマジックナンバーにあたります。

例えばデータベースに入っている10人分の顧客データに対してループするコードだったとしましょう。
もし顧客が増えたら、上のコードの”10″の部分を書き換えなければいけません。
そのため、人数であればlengthなどを使用するなどでマジックナンバーを防ぐことができるわけです。

また、私が指摘されたものとして、”文字版マジックナンバー”が挙げられます。
具体例は秘密保持のため出せませんが、頻繁に変更が入る商品名のような文字列をコード内に入れていました。
数字であれば何かの数を取得なり算出すれば良いですが、文字の場合はどうすれば良いでしょうか。
時と場合に寄ることは当たり前ですが、私は、正規表現を用いました。
正規表現であれば文字列のどこが変動するか把握しておけばその部分にワイルドカードを当ててしまえば良いので、文字版マジックナンバーを防ぐことができました。

さて、ここでの本質は”後から変更が入る可能性のものを直接書き込まない”部分にあります。
マジックナンバーを知った後であれば、メンテナンス性という観点は持てるはずですから、
“後からこの部分を修正することになったら？”という意識は忘れないようにすることをお勧めします。

④1つのメソッドが担う役割

メソッドの切り出しを覚えた私は、”どのような単位で切り出せば良いのか”と思いました。
最初はなんとなくでメソッドを分け、運用していましたが、後の指摘である概念を学びました。
それは”メソッドにも責任範囲がある”ということです。

例えば、何かのパラメータが正常かどうかをチェックするメソッドがあったとします。
おそらくメソッド名はisParamOk()とかになるでしょう。
ここで、このメソッドが以下のような処理だった場合を考えます。

isParamOk(param){
  if(param=="ok") return true;
  else{
    param.addNgCount();
    return false;
  }
}

渡されたパラメータがokか比較し、okだった場合はtrueを返す。
そうでなければNGのカウントを増やし、falseを返すメソッドです。
この程度ならもしかしたら問題は起きないかもしれませんが、このメソッドは”何を任されているメソッド”かを考える必要があります。

後からこのコードを読んだ人がいたとして、isParamOk()が呼び出されていたらどう思うでしょうか。
おそらく、okかどうかのチェックのみを行うと判断するはずです。
しかし、実際には、okでなかった場合にNGのカウントを増やす部分まで行われてしまっています。
チェックのみを行うはずのメソッドが、パラメータの中身の更新まで行ってしまっているわけですね。
このように、メソッドが担う役割は、可能であればその名前から大きく逸脱せずに、複数持たないことが好ましいと言えます。

3.コーディングにおける品質とは

ここまでは指摘それぞれにおける本質に触れてきましたが、これらを包括するとどのようなものが見えてくるでしょうか。
“良い品質のコード”の条件は現場や職種によって異なることは前提ですが、
私が考えるには”できるだけ無駄がなく、誰にでも伝わりやすいコード”であるように感じます。
自分だけが読めるコード、ではなく、後で別の人がメンテナンスできるように、他の人から見たコードを意識することがコーディング品質の向上に繋がる第一歩だと思います。

4. まとめ

私がもらった指摘はこんな量ではありませんでしたが、本質として注意しなければならないと感じたものを中心にピックアップしました。
品質の良いコードを意識したコーディングこそが「品質の良いコーディング」であると私は考えます。
新人エンジニアの皆様、これからたくさん注意されることがあると思いますが、品質の良いコーディングができるよう一緒に頑張っていきましょう。

ブログ記事一覧へ戻る

satokのブログ

ブログ内検索