「最近、職場で興味深い状況に遭遇しました。コードレビューを行い、セキュリティチェックを挿入しました。コンストラクターの引数にnullをチェックし、プロパティから返されたnull値をチェックします。 また、自分の仮定を明示的に述べるために使用したプライベートメソッドのステートメントもありました。
「チームメイトの間で一般的に行われているのは、チェックを省略して転倒を許可することです。 正直に言うと、私はこの概念に苦労しています。なぜなら、私はすでに防御的なプログラミングによる開発に慣れていて、それが良い習慣だと思っていたからです。 私はこれがほとんどのマニュアルとブログ投稿に当てはまると確信しています。
「コードを失敗させてからスタックトレースをチェックするよりも、防御的なスタイルでプログラムする方が良い理由についてアドバイスをいただけますか？」
一般的に言えば、防衛的プログラミングは他のアプローチよりも 劣るとは思いません。 いつものように、さまざまな要因が問題の解決に影響を与えるので、簡単な答えはこれです： それはすべて状況に依存します。
もちろん、このような答えは、質問に答えられるまで役に立たない： この依存関係はどのような状況から生じるのか？ この記事では、いくつかの要因を検討します。 ただし、この記事がトピックの包括的なプレゼンテーションであることを述べるつもりはありません。
コードをフォールさせます。
すぐに例外をスローしたい場合、防御プログラミングの価値は何ですか？ コードを落とすのと同じくらい良い解決策ではないでしょうか？ いくつかのサンプルコードを見てみましょう。
public class ProductDetailsController { private readonly IUserRepository userRepository; private readonly IProductRepository productRepository; private readonly ICurrencyExchange exchange; public ProductDetailsController( IUserRepository userRepository, IProductRepository productRepository, ICurrencyExchange exchange) { this.userRepository = userRepository; this.productRepository = productRepository; this.exchange = exchange; } public ProductDetailsViewModel GetProductDetails(string userId, string productId) { var user = this.userRepository.Get(userId); var targetCurrency = user.PreferredCurrency; var product = this.productRepository.Get(productId); var convertedPrice = this.exchange.Convert(product.UnitPrice, targetCurrency); return new ProductDetailsViewModel { Name = product.Name, Price = convertedPrice.ToString() }; } }
この単純な
ProductDetailsController
クラスでは、
GetProductDetails
メソッドが
ProductDetailsViewModel
インスタンスを作成し、リポジトリの注入によりユーザーおよび製品情報を取得し、製品の価格をユーザーの優先通貨に変換し、画面に表示されるはずの製品データを返します。 この記事の目標を達成するために、
null
をチェックする問題のみに焦点を当てましょう。
GetProductDetails
メソッドで失敗したスクリプトを通過できる呼び出しの数は？ null参照になるオブジェクトはいくつありますか？
かなり多くのことが判明しました。 依存関係から分離されていても、この小さなコードは少なくとも6つのケースで
NullReferenceException
をスローできます。 使用中のシステムからエラー報告を受け取っていると想像してください。 スタックトレースは
GetProductDetails
メソッドを指し、スローされる例外のタイプは
NullReferenceException
です。 null参照を持つ6つの可能なオブジェクトのどれがエラーの原因ですか？
システムを落とすことで、この質問に答えることが難しくなります。 そして、これは単なるケーススタディであることを忘れないでください。 私が出会った悪用コードのほとんどは5行または6行以上のコードであったため、エラーの原因を突き止めると、すぐに干し草の山で針を見つけるようなものになります。
コードを落とすだけではあまり役に立ちません。 明らかに、非常に短いメソッド（私が強く推奨するプラクティス）を記述する場合、発生する問題はそれほど緊急ではありませんが、メソッドが長くなればなるほど、専門家は少なくなります。
保護レスキュープログラミング？
明示的なゲートキーパーを
null
追加することにより、より説明的なエラーメッセージをスローできます。
public class ProductDetailsController { private readonly IUserRepository userRepository; private readonly IProductRepository productRepository; private readonly ICurrencyExchange exchange; public ProductDetailsController( IUserRepository userRepository, IProductRepository productRepository, ICurrencyExchange exchange) { if (userRepository == null) throw new ArgumentNullException("userRepository"); if (productRepository == null) throw new ArgumentNullException("productRepository"); if (exchange == null) throw new ArgumentNullException("exchange"); this.userRepository = userRepository; this.productRepository = productRepository; this.exchange = exchange; } public ProductDetailsViewModel GetProductDetails( string userId, string productId) { var user = this.userRepository.Get(userId); if (user == null) throw new InvalidOperationException("User was null."); var targetCurrency = user.PreferredCurrency; var product = this.productRepository.Get(productId); if (product == null) throw new InvalidOperationException("Product was null."); var convertedPrice = this.exchange.Convert(product.UnitPrice, targetCurrency); if (convertedPrice == null) throw new InvalidOperationException("Converted price was null."); return new ProductDetailsViewModel { Name = product.Name, Price = convertedPrice.ToString() }; } }
このコードは良いですか？ エラーの原因の分析に関しては、はい。 このバージョンのコードでは、オペレーティングシステムからエラーメッセージを受け取った場合、例外メッセージにより、プログラムが発生した
null
6つの可能性のある状況のどれかが
null
ます。 サポートモードの場合、2つのバージョンのうちどちらが望ましいかがわかります。
ただし、読みやすさの点では、すべてが悪化しています。
GetProductDetails
メソッド
GetProductDetails
、5行から11行のコードに延長されました。 保護プログラミングにより、行数が2倍以上になりました！ メソッドの本体を通る通路は保護構造にdrれているため、メソッドはさらに読みにくくなりました。 あなたが典型的なプログラマーである場合、コードを書くよりも読むのがはるかに多いので、コードを読みにくくする習慣は、あなたをすきのように感じるはずです。 多くのプログラマーが防御的プログラミングを悪い習慣と見なしていることは間違いありません。
持続可能性。
問題とその解決策に影響する要因のバランスを取ることは可能ですか？ はい、可能ですが、その方法を理解するには、問題の根本原因を理解する必要があります。 最初のコード例は特に複雑ではありませんが、そうであっても、このコードが失敗する場合が多くあります。 null参照に関しては、その理由は言語の設計の誤りですが、一般的には、入力データを信頼できるかどうかが問題です。
IUserRepository.Get
呼び出しを介した戻り値も（間接的に）入力されます。
プログラムが動作する環境に応じて、入力データを信頼する能力があるか、そのような機会がありません。 あなたのソフトウェアが「荒野」で運用されている状況をちょっと想像してみてください。 ソフトウェアは、再利用可能なライブラリまたはフレームワークである場合があります。 この場合、入力をまったく信頼できません。 もしそうなら、持続可能性の原則を適用し、インプットだけでなくアウトプットに関しても防御的なスタイルで行動していると確信し続けることができます。 つまり、null参照（または他の悪魔的な値）を他の対話コードに渡したくないということです。
前述のコード例では、たとえば、
userId = null,
場合
userId = null,
または（より洗練された）
user.PreferredCurrency = null.
場合、null参照をその依存関係に渡すことができ
user.PreferredCurrency = null.
したがって、安定性の原則に従って、さらに防御的な式を追加する必要があります。
public class ProductDetailsController { private readonly IUserRepository userRepository; private readonly IProductRepository productRepository; private readonly ICurrencyExchange exchange; public ProductDetailsController( IUserRepository userRepository, IProductRepository productRepository, ICurrencyExchange exchange) { if (userRepository == null) throw new ArgumentNullException("userRepository"); if (productRepository == null) throw new ArgumentNullException("productRepository"); if (exchange == null) throw new ArgumentNullException("exchange"); this.userRepository = userRepository; this.productRepository = productRepository; this.exchange = exchange; } public ProductDetailsViewModel GetProductDetails( string userId, string productId) { if (userId == null) throw new ArgumentNullException("userId"); if (productId == null) throw new ArgumentNullException("productId"); var user = this.userRepository.Get(userId); if (user == null) throw new InvalidOperationException("User was null."); if (user.PreferredCurrency == null) throw new InvalidOperationException("Preferred currency was null."); var targetCurrency = user.PreferredCurrency; var product = this.productRepository.Get(productId); if (product == null) throw new InvalidOperationException("Product was null."); if (product.Name == null) throw new InvalidOperationException("Product name was null."); if (product.UnitPrice == null) throw new InvalidOperationException("Unit price was null."); var convertedPrice = this.exchange.Convert(product.UnitPrice, targetCurrency); if (convertedPrice == null) throw new InvalidOperationException("Converted price was null."); return new ProductDetailsViewModel { Name = product.Name, Price = convertedPrice.ToString() }; } }
これは、明らかに、防御的なスタイルを使用した前の例よりもひどいように見えます。 これで、自分自身だけでなく、相互作用するコードも保護できます。 称賛に値するが、完全に読めない。
今まで、「荒野」に住むコードを書くとき、そのような防御的なプログラミングはまさに私がやっていることですが、私はまだ、最初にコンパイルしてすべての入力データをチェックしてから、このデータをすべてのロジックを実装する別のクラスに渡します。
保護されたエリア。
コードが保護されたエリアにある場合はどうなりますか？ コードが環境で動作する場合、相互作用するすべてのコードは、あなたと同僚によって書かれた同じコードベースの一部ですか？ 特定の一貫した規則に従うことで相互に信頼できる場合は、ほとんどの保護構造を省略できます。
私が働いていたほとんどのチームでは、私たちは常に持続可能性の原則を使用していると考えていました。 実際には、これは
null
有効な戻り値になること
null
ないことを意味し
null
。 クラスメンバーが
null
返す場合、バグはこのクラスのコンシューマではなく、このクラスにあります。 この規則を考えると、前のコードはこれに短縮できます。
public class ProductDetailsController { private readonly IUserRepository userRepository; private readonly IProductRepository productRepository; private readonly ICurrencyExchange exchange; public ProductDetailsController( IUserRepository userRepository, IProductRepository productRepository, ICurrencyExchange exchange) { if (userRepository == null) throw new ArgumentNullException("userRepository"); if (productRepository == null) throw new ArgumentNullException("productRepository"); if (exchange == null) throw new ArgumentNullException("exchange"); this.userRepository = userRepository; this.productRepository = productRepository; this.exchange = exchange; } public ProductDetailsViewModel GetProductDetails( string userId, string productId) { if (userId == null) throw new ArgumentNullException("userId"); if (productId == null) throw new ArgumentNullException("productId"); var user = this.userRepository.Get(userId); if (user.PreferredCurrency == null) throw new InvalidOperationException("Preferred currency was null."); var targetCurrency = user.PreferredCurrency; var product = this.productRepository.Get(productId); if (product.Name == null) throw new InvalidOperationException("Product name was null."); if (product.UnitPrice == null) throw new InvalidOperationException("Unit price was null."); var convertedPrice = this.exchange.Convert(product.UnitPrice, targetCurrency); return new ProductDetailsViewModel { Name = product.Name, Price = convertedPrice.ToString() }; } }
それは良いですが、まだ十分ではありません...しかし、待ってください：読み取りプロパティも戻り値なので、どちらもチェックしないかもしれません：
public class ProductDetailsController { private readonly IUserRepository userRepository; private readonly IProductRepository productRepository; private readonly ICurrencyExchange exchange; public ProductDetailsController( IUserRepository userRepository, IProductRepository productRepository, ICurrencyExchange exchange) { if (userRepository == null) throw new ArgumentNullException("userRepository"); if (productRepository == null) throw new ArgumentNullException("productRepository"); if (exchange == null) throw new ArgumentNullException("exchange"); this.userRepository = userRepository; this.productRepository = productRepository; this.exchange = exchange; } public ProductDetailsViewModel GetProductDetails( string userId, string productId) { if (userId == null) throw new ArgumentNullException("userId"); if (productId == null) throw new ArgumentNullException("productId"); var user = this.userRepository.Get(userId); var targetCurrency = user.PreferredCurrency; var product = this.productRepository.Get(productId); var convertedPrice = this.exchange.Convert(product.UnitPrice, targetCurrency); return new ProductDetailsViewModel { Name = product.Name, Price = convertedPrice.ToString() }; } }
これで十分です。コードの初期状態にほとんど戻っているからです。 唯一の違いは、各メンバーの最初にゲートハウスが存在することです。 持続可能性の原則に従うと、ほとんどのメンバーが同じことを考え始めます。 少し後に、それに慣れると、気づかないようになります。 私はそれらを各メンバーの前置きのようなものと考えています。 コードリーダーとして、コードの読み取りを妨げる保護チェックを中断することなく、ゲートをスキップしてロジックのフローに集中できます。
カプセル化。
null
参照を返すことがエラーの場合、
User
クラスまたは
Product
クラスはどのように持続可能性の原則に従うことができますか？ 同じように：
public class User { private string preferredCurrency; public User() { this.preferredCurrency = "DKK"; } public string PreferredCurrency { get { return this.preferredCurrency; } set { if (value == null) throw new ArgumentNullException("value"); this.preferredCurrency = value; } } }
User
クラスがその不変式を保護する方法に注目してください。
PreferredCurrency
プロパティを
null
することはできません。 この原則は、別の名前： encapsulationでも知られています 。
おわりに
いつものように、問題の根底にある要因を理解するか、コードベースを理解することが役立ちます。 コードが保護された環境に住んでいるときよりも「荒野」に住んでいる場合は、はるかに防御的な構造を書く必要があります。 今日まで、私はあなたが乱雑なコードを書くことで得ることができると信じるのは誤りだと考えています。 私たちは皆、 レンジャープログラマである必要があります。
構造化されていないセキュリティコードは可読性を損ないます。 セキュリティコードは、スパゲッティコードを記述するための別の言い訳にすぎません。 対照的に、構造化された防御プログラミングはカプセル化です。 私は私が好むものを知っています。