⛳

実務で「こういうコードは気持ち悪い」と指摘されたことについて! とにかく読みづらいクソコードを解説!未経験、経験浅いエンジニアは必ず読め

2023/10/25に公開

新人プログラマ応援

プログラマー脳

tech

概要

実務の時に「ちゃんと動いたし、コードも綺麗だ。これならレビューも通るだろう!」
と思ってプルリクを出した時にリーダーから言われたことがあります。

「確かにちゃんと動いているし、動作上問題はないんだけど、このコードなんか気持ち悪いんだよね」

と言われました。何が気持ち悪いんだと思いましたが、言われてみると納得でした。
未経験の方にはイメージがつかないかもしれないですが、
コードの品質ってすごい大切なんです！動けばOKではないです！

この記事の対象

1.開発エンジニアを目指している未経験の方
2.まだ経験が浅い方(実務経験半年未満)の開発エンジニア

具体的にどんなコード

コードは自分が現場から離れてからも他の人が見てもわかるようにしなければいけません。
「コードの品質に妥協してはいけない。自分達はプロなんだ」
みたいなことを言われたので実際に例を見ていきます。

RSpecでテストの実装をしていたのでその時のコードを一部参考に見ていきます。

 let(:data) { ✖️✖️✖️PdfService::CREATE_DATE }

    context '正しい値がある時' do
      it '新しくデータが作成されること' do
        expect do
          User::Pdf.create(user_data, date)
  end.to change { Dir.glob("#{〇〇}/#{■■}/#{▲▲}.pdf").count }.by(1)
        File.delete("#{〇〇}/#{■■}/#{▲▲}.pdf")

この部分をレビューで気持ち悪いと言われました。
どこかというと

("#{〇〇}/#{■■}/#{▲▲}.pdf")

の部分で.pdfが２回使われています。これだと毎回毎回.pdfを書いていくことになります。
このファイルのプログラムは小さかったのですが、プログラムがもっと巨大になると
〇〇.pdfが10個も20個もできてコーディングもめんどくさいし、読みづらいです。

.pdfを使わなくて済むような実装をするようにレビューをいただきました。
すごい細かいんですが、こういう細かいことが大切です。

コードの改善提案まで出せた時

この指摘を受けて自分でもコードの改善案を出せました。

    let(:data) { ✖️✖️✖️PdfService::CREATE_DATE }
    let(:file) { "#{〇〇}/#{■■}/#{user_data[:user_id]}.pdf" }

    it '正しい値が入力された場合、帳票が作成されること' do
      expect { User.create(user_data, date) }.to change { Dir.glob(file) }
    end

    it '正しい値が入力されなかった場合、帳票が作成されないこと' do
      expect { User.create(nil, date) }.not_to change { Dir.glob(file) }
    end
  end
end

ここではUser.createが２回繰り返されていました。
こういう細かいとこに違和感がある感覚でないととダメなんですね!

こんな細かい指摘も!

あとこういう場面でも指摘が入りました。endを52行目のような状態でも指摘が入りました。
なぜかわかりますか。endの横に空欄があることが原因です。
動作上問題無くてもコードの品質上ふさわしくないのです。

スクリーンショット 2023-10-04 1.57.34.png

Youtubeでクソコードを紹介

リーダブルコードについて

コードの品質ならリーダブルコードを読んでみてください！

内容は以下の記事にまとめてあります。

まとめ

このように細かいとこにまで注意を向けないとやっていけないのが開発エンジニアです。
開発業務をやることで几帳面になっていくと感じました。
僕は大雑把な性格だったのですが、やっているうちに細かいとこに目が行くようになりました。

こういう指摘は実務をやらないとなかなか指摘をしてくれません!
未経験の方は早く現場に入りましょう!

補足

この記事は2023年10月3日に投稿された記事です。訳あってこちらに投稿することにしました。