【チェックシート】システム開発において、SAPUI5で実装した際のコードレビューでよくある指摘観点をまとめる

# はじめに
SAP社のエンタープライズPaaS製品である、SAP Business Technology Platform（BTP） でアプリケーション開発をしている@fussasyです。開発プロジェクトを経験する中で、チームメンバーのコードレビューをする機会が多くありました。やはり、コードレビューでは似たような指摘も発生してしまい、手間に感じている部分もありましたので、まとめてしまいたいと思いました。
当記事ではレビュイーがコードレビューに向かう前の事前チェック資料、もしくはレビュアーがレビューをするための参考資料となるような一覧整理資料を目指しています。アプリケーション開発する際、品質の向上に繋がれば嬉しく思います。なお、この記事については随時、更新すると思います。

# チェック観点
### 公式ガイドライン
以下、ガイドラインを一読のうえ、SAP UI5における実装を厳守しているか

Development Conventions and Guidelines
https://help.sap.com/saphelp_uiaddon10/helpdata/en/75/3b32617807462d9af483a437874b36/frameset.htm
　┗　JavaScript Coding Guidelines
　┗　SAPUI5 Control Development Guidelines
　┗　Product Standards and Acceptance Criteria
　┗　File Names and Encoding
　┗　Tools

### 仕様・規約とのつきあわせ
提供されている「命名規約」と「コーディング規約」に従って記述していること
コーディングの際、仕様が不明瞭なものについては、関係者にQAを実施し回答を刈り取っていること
コーディングの際、関係者にQAを実施し、回答を刈り取ったものについて、証跡を一覧として提示できること
コーディングの際、スケルトンを利用する場合には、バグ発生時における責任の所在が明確になっていること
詳細設計書（基本設計書）に記述された処理をコーディングしていること
詳細設計書（基本設計書）について、レビュアに対してコードへの反映箇所を説明できること
詳細設計書（基本設計書）と突き合わせがしやすいよう、コメント文を記述したこと

### 単体テスト観点とのつきあわせ
単体テスト仕様書がある場合、各テスト観点については事前にデバッグで検証しているか

### よくある指摘
不要なコード、コメント、関数等は残っていないこと
開発時のデバッグコードは残っていないこと
警告（エラー）箇所は消えているか。消えていない場合、デプロイしても問題ないか確認していること
警告（エラー）は、フロントエンド側かバックエンド側のどちらで出力するか、関係者にQAを実施し回答を刈り取っていること
空白行は、2行以下となっていること
特殊符号の編集時には、全角・半角を考慮していること
メッセージテキストはハードコーディングをしていないこと（「MESSAGE 'エラー発生' 」等)
多言語対応が必要となる可能性がある場合、i18nで定義していること（画面のラベル名　等）
比較演算子に「===」「!==」を使用していること（「==」や「!==」はNG）
複数の非同期処理を、直列や並列処理させる際、意図した通りに動いていること（Promiseチェーン：promise～～then～resolve、reject）
入力項目に入力チェックを入れる際、タイミングは仕様通りであること
入力項目に入力チェックを入れる際、タイミングに入力項目ごとに不整合はないこと
必須チェックと入力チェックを一括で処理していないこと（二重エラーが発生するため）
条件分岐処理で、例えば複数の要素（AとB）で条件を指定する場合、AとBを並列で条件処理とするか、A→Bの順で条件処理とするか誤っていないこと
条件分岐処理において、「0」か「1」でしか判断しないのであれば、「TRUE」か「FALSE」にしていること
条件分岐処理において、正常系＝「TRUE」、および異常系＝「FALSE」でチーム内メンバー全員にて、統一していること
条件分岐処理において、ネストは限界まで浅くしていること（深さ＝6以上になると可読性が悪化）
条件分岐処理において、必須チェック後に処理から抜けないことにより、二重エラーを発生させていないこと
比較処理において、定数を前後どちらにするか、チーム内メンバー全員にて、統一していること
ポップアップマネージャに、複数メッセージ（「Info」「Warning」「Error」等）を表示する場合、各処理で配列に詰めた後に繰り返し処理でまとめて表示していること
メッセージダイアログでは、メッセージ種別（「Info」「Warning」「Error」等）について、アイコンとメッセージ内容は適切になっていること
暗黙の型変換が発生している箇所はないこと
変数は可能な限り減らしていること
変数宣言時、変数名は使用箇所や意図が分かる名前になっていること（「id1」「id2」等になっていないこと）
変数宣言時、変数名は似たような名前を多用していないこと（見間違いの原因となる）
変数宣言時、ローカルで宣言すべきものを、グローバルで宣言していないこと
変数宣言時、初期値は何も入れないとundefinedとなる。初期値またはnull入れていること
変数宣言時、「var」と「let/const」は混在していないこと
変数宣言時、やむを得ない理由を除き、再代入と再宣言はしていないこと
変数宣言時、「let」と「const」において、やむを得ない理由を除き、「const」を利用していること
無意味なコメントは入れていないこと（Promise処理に「ここからプロミス処理」と入れる等）
冗長処理になっている箇所はないこと
関数の引数と返値に同一の配列を使用していないこと
debugge文を利用していないか（非推奨）、もし利用してしまっていた場合、消し忘れないこと
インデントは統一していること（規約も確認すること）
1行あたりの処理が、長すぎる状態となってないこと（規約も確認すること）
関数の処理の一連の流れが、長すぎる状態となってないこと（規約も確認すること）
配列の添え字を指定した場合、必ず想定のデータがそこに入ってくるか確認していること（「oSmartFilterXYZ0」の場合とか、0番目に必ず目的のデータが入るかどうか）
SmartTableを利用している場合、IgnoreFieldsに、画面非表示項目を明示的に記載しているか確認していること
ODataからデータを取得している場合、JSTになっているか確認していること（ABAP側はUTCだが、CDS経由すると自動で+9:00される）

# おわりに
SAP UI5もSPA系フレームワーク（例：React、Angular、Vue.js等）であり、コンポーネントライブラリも一通り揃っていたりするので、先の例で示した、JavaScriptのフレームワークで開発経験があれば、何を当たり前を仰る…？といったものも多くあるかもしれません。
色々と書きましたが、結局のところSAP UI5でフロントエンド側を開発する場合、冒頭にて紹介したSAP UI5公式ガイドライン（Development Conventions and Guidelines）を読み込むのが早いと思います。実際、弊社にて命名規約やコーディング規約を作成する際には、当資料を多分に参考にさせてもらいました。また、レビュー以前に各種フォーマッターにこれらの設定を盛り込んでおけば、細かくコードを見る必要もなくなるので開発効率も上がると思います。