言うまでもないですが、完璧なコードはありません。完璧と思ったコードに不具合があることは良くあります。そして、問題があったら調査に駆り出されるのは、実装した「あなた」か将来の保守担当者です。
将来の不具合調査を楽にするために、今できる最低限の実装をここでは紹介したいと思います。
最初の実装
ではまず、このgetCustomerというメソッドを例にしてみます。
このメソッドはcustomerIdとtenantIdを引数にしてCustomerオブジェクトを返すメソッドです。
Customerは特定のtenantに属しており、引数で渡されたtenantIdに必ず所属している必要があります。
所属が違う場合には、不正はアクセスとみなして、nullを返却します。
多分プログラミング経験が浅い方以下のように書くでしょう。
getCustomer(customerId: string, tenantId: string): Customer | null {
let customer = this.customerRepository.get(customerId);
if (customer.tenantId !== tenantId) {
return null;
}
return customer;
}
レビューしてみよう
さて、このメソッドをレビュワーの視点で見てみましょう。
この場合には以下のようなケースが想定されます。あなたは以下の回答を用意できていますか？
- customerIdがnull/undefinedのケース
- tenantIdがnull/undefinedのケース
- customerIdのCustomerがデータベースに存在していないケース
- データベースが停止しているなどでコネクションが張れてないケース
- customerがtenantIdに所属していないケース
これらの問題が本番環境で発生するたびに、あなたは調査しないといけないわけです。
もちろん、発生可能性は低いと思います。なのでこのコードでも良しとするケースが大半でしょう。
ですが、引数がブラウザなどのクライアントから渡ってくる場合には、引数が不正になる可能性は多いにあります。意外とDBを直接編集されて、Customerテーブルに変なデータが入ってくる場合もあります。
そのたびに、あなたはログを収集して、手がかりを探して、右往左往するのです。
ではどうするか？
最低限「問題が発生したときに追跡しやすくしておきましょう」です
最低限の実装をしておこう
追跡するためにはログが必要です。アプリケーションログの目的はいろいろあるのですが、一番大きい目的は「不具合調査」です。
ログを見ただけでどこがいけないのか、どこまでOKでどこからNGなのかを切り分けるのに非常に役に立ちます。
さて今回のケースでは、これくらいは実装をしておいて欲しいところです。
getCustomerB(customerId: string, tenantId: string): Customer | null {
if (!customerId || !tenantId) {// 引数チェック
throw new Error(
`Illegal customerId or tenantId, customerId=${customerId}, tenantId=${tenantId}`
);
}
let customer = null;
try {
customer = this.customerRepository.get(customerId);
} catch (e) {// DB コネクションチェック
this.logger.error(`Failed to get customer customerId=${customerId}, tenantId=${tenantId}`);
throw e;
}
if (!customer) {// Customer存在チェック
this.logger.warn(`Customer does not exist customerId=${customerId}, tenantId=${tenantId}`);
return null;
}
if (customer.tenantId !== tenantId) {// Tenant 所属チェック
this.logger.warn(
`Customer does not belongs to the tenant customerId=${customerId}, tenantId=${tenantId}, customer.tenantId=${customer.tenantId}`
);
return null;
}
return customer;
}
まず引数のチェックは行っておきます。確実に不正な値が渡ってこないことが明らかな場合や、呼び出し元でチェックが行っているとわかっている場合には、冗長になるので不要な場合も多いです。
ここまでやっておけば、不具合調査でログを収集しても、すぐに問題箇所がわかるわけです。調査コストが下がりますね。
ここで上記ケースの場合には、nullを返すの？と思ったアナタは一歩前進していますね。
null safetyな実装というのもあるので、調べてみて下さい。世の中のエラーの大半はヌルポです。
そして大事なこと。
ログには必ずパラメーターを出力するようにしておきましょう。
これが後で自分を助けることに繋がります。
最後に、契約プログラミング、防御的プログラミングという概念もあるので、参考にしてください。
言語ごとの特徴もあるので、実装方法は異なりますが、概念は大きく変わらないと思います。
以上