Webエンジニア1年目にされたコードレビューまとめ【フロントエンド編】

こんにちは。ひろき(@hiro_az4511)です。

Webエンジニアとして働き始めてから、早くも2年目になりました。

この記事では、私が実際に入社してから1年目に先輩・上司にしていただいたコードレビューをまとめてみようと思います。

取り扱うレビューは、私がエンジニアとして働き始めてから、大体半年以内にいただいたレビューです。

そのため、全体的に初歩的な内容にはなりますが、これからWebエンジニアとしてキャリアをスタートしようと考えている方などの参考になればと思います。

1年目のタスク概要

はじめに、私が1年目で携わらせていただいたタスクは主に下記の通りです。(括弧内に使用技術スタック記載)

私の1年目の主なタスク
  • Webメディアのフロントエンド・サーバーサイド実装およびABテスト
    (Ruby[Ruby on Rails] / Javascript[React.js. jQuery] / HTML/ CSS[SCSS] / AWS[EC2/RDS/S3/Cloudfront/Elastic Beanstalk/ ElastiCache] / Google Optimizer)
  • 新コーポレートサイトの立ち上げ
    (AWS[EC2/RDS/S3/Cloudfront] / Firebase / WordPress REST API / Javascript[Nuxt.js] /HTML / CSS[SCSS])
  • 社外向けブログ立ち上げ
    (AWS[EC2/S3/Cloudfront/ RDS / Route 53] /HTML / CSS[SCSS] / Javascript[jQuery])
  • Slack BOTの開発
    (Slack API / Google App Script / Salesforce API / Google Analytics)
  • 社内分析ツール開発
    (GCP(GAE/Bigquery)/ Golang / Twitter API)

1年目はフロントエンド・サーバーサイド・クラウドインフラと、様々な技術に触れることができました。

本記事では、これらのタスクで先輩にいただいたコードレビューの中で、より汎用的かなと思う内容を選び、まとめていきます。

長くなりそうなので、フロントエンド編、サーバーサイド編、番外編に分けようと思います。

フロントエンド編(HTML/CSS/Javascript)

ここでは、コーポレートサイト(Wordpress API + Nuxt.js), ウェブメディア(Rails)のフロント側を実装していた時にいただいたレビューをまとめていきます。

インデントの不揃い、数に統一性がない

これは基本中の基本ではあるのですが、独学で学び、チーム開発をしたことがなかった私が、エンジニアのキャリアの最初にもらったレビューです。

最低限インデントがしっかり揃っているか確認してから、プルリクエストを出すようにしましょう。

なお、インデントの数に関しては、言語やフレームワーク、プロジェクトや会社のルール、宗教(?)によって異なってくるので、先輩に聞くか、既存のコードに合わせるようにすれば問題ないかと思います。

余談ですが、アメリカのドラマ「シリコンバレー」の有名なシーンで、「タブ派かインデント派か」という言い合いがあります。

このようなエンジニア界隈の宗教戦争は多数存在するようですね、、、(笑)

CSSのプロパティ重複、不要なプロパティの存在

名著の達人プログラマーで提唱されていて、Ruby on Rails の基本理念のひとつともされている、DRY( Don’t Repeat Yourself)にあるように、基本的にプログラムの中で、同じことを重複して記述することは避けるべきです。

DRY原則を破るということは、同じ知識を2箇所以上に記述することです。この場合、片方を変更するのであれば、もう片方も変更しなければならないのです。

達人プログラマー―システム開発の職人から名匠への道( ANDY HUNT / DAVE THOMAS)

私は、CSSの同じプロパティを、ファイルのあちこちに記述し、お互いを打ち消し合うようなコードを書いていました。

これだと、メンテナンスをする際に、変更しなければならない箇所が多くなり、後々自分やチームメンバーがそのコードを変更する時に大変になります。

さらに、不必要なプロパティを記述している箇所も、かなり多くありました。

私にとってCSSはかなり難しく(今でも)、その挙動に非常に苦しみました(今でも苦しんでる)。

CSSのプロパティの重複や不必要な箇所を極力無くすためには、定期的に書いたコードを全て見直し、プロパティを消しても問題ないかをチェックするといいと思います。

クラス名/変数名/関数名などの命名規則について

クラス名や関数名、メソッド名等の命名規則も、言語やフレームワークによってその慣習が違います。

また、個人の思想によって大きく違いが出るところだともいます。

私はこれらの命名に悩む時が時々あります。いや、かなりあります。

先輩からの愛のあるコードレビューの中で私が学んだ、命名をする際に特に気をつかうべき事は下記の2点です。

命名のポイント
  1. 理解しやすい・具体的である
  2. 一貫性・統一性がある

1. 理解しやすい・具体的である

誰が見てもその名前が何をしているのかがわかるか。抽象度が高くないか。

2. 一貫性・統一性がある

プロジェクトを通して命名に一貫性・統一性があるかはかなり重要です。

例えば、キャメルケース、スネークケース、ケバブケースのどれを使うか。

単数形にするか複数形にするか。名詞を使うか動詞を使うか。等々。

今更紹介するまでもないほど有名な書籍ですが、これらの規則について考えるべき事は、「リーダブルコード」にも書いてあり、私自身、キャリア1年目に読んで非常に役に立ちました。

またクラス名に関しては、CSSの設計手法によってもその規則が異なるため、どんな手法があるかや、それぞれのメリット・デメリットも知っておくべきです。

下記の本は、先輩からオススメしてもらい、非常にわかりやすかったのでぜひ読んでみてください。

不要なコメントが残っている

コメントは目的を持たせ、コードの書き手の意図を読み手に知らせるために書きます。

不要なコメントは、プルリクエストをする前に消しましょう。

「適切なコメントとはなんぞや?」に関してもリーダブルコードに書いてあります。

適切なビューヘルパーを使う

開発で使っていたフレームワーク(Rails)ではViewをよりシンプルに書くため、ビューヘルパーが用意されています。

その便利なヘルパーを使用することによって、より可読性の高く、DRYなコードを書くことができます。

この事を全く理解していなかった私は、ヘルパーを使った方がシンプルにかける場面でもHTMLのタグを使用してコードを冗長にしていていました。

もちろん、Railsに限った話ではなく、使う言語やフレームワークによって、”お作法”的なものが存在するので、調べてみてください。

CSSのプロパティがブラウザ対応しているか、実機確認

フロントエンドの開発において、「各デバイス・各ブラウザでしっかり動作するか」のチェックは欠かせません。

例えば、下記はCSSのunsetというプロパティのブラウザ対応表を表してます。(参照)この場合は、Internet Explorerに対応してません。

対応していないプロパティを使っていると、デザインが崩れてしまうので、ここもよくチェックする必要があります。

ただ、どのCSSプロパティがどのブラウザに対応しているかを全て覚えるのは無理(+効率が悪い)だと思うので、しっかり公開前に各ブラウザでチェックするようにしましょう。

特殊文字に適切なエスケープ処理をする

特に& ” < > などの記号や、スペースをそのまま表示したい時などによく使います。

エスケープをしないと、クロスサイトスクリプティング(XSS)という攻撃の標的になってしまうことがあるようです。

細かくは、下記の記事がよくまとめられているので、読んでみてください。

また下記のページには、特殊文字のコードがまとめられていますので、必要な時にここをチェックして使うといいかと思います。

まとめ

細かいところをあげればまだまだあるのですが、以上がフロントエンド開発で私がウェブエンジニア1年目に、優しい先輩方からいただいたコードレビューのまとめでした。

これからエンジニアデビューをしようとしている方は、頭の片隅に入れておくと良いのかなと思います。

いつになるかわかりませんが、次はサーバーサイド編もまとめていきます。

コメントを残す

メールアドレスが公開されることはありません。 * が付いている欄は必須項目です