Один из моих читателей, Барри Гайлз, недавно написал мне и задал достаточно интересный вопрос, который, по моему мнению, достоен обсуждения:
«Недавно я столкнулся с одной интересной ситуацией на работе: я производил ревью кода и вставил защитные проверки – одну для проверки аргумента конструктора на 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? Как много объектов могут быть нулевыми ссылками?
Довольно много, как выясняется. Даже отделённый от своих зависимостей, этот небольшой кусочек кода может выбросить NullReferenceException, как минимум в шести случаях. Представьте, что вы получаете отчёт об ошибках с вашей системы, находящейся в эксплуатации. Трассировка стека указывает на метод GetProductDetails и типом выброшенного исключения является NullReferenceException. Какой из шести возможных объектов с нулевой ссылкой стал причиной ошибки?
Давая системе просто упасть, нам будет трудно ответить на этот вопрос. И не забывайте, что это всего лишь учебный пример. Большая часть эксплуатируемого кода, который я встречал, имел более 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-ссылкой столкнулась ваша программа. Если бы я находился в режиме поддержки, то я бы знал какую версию из представленных двух я бы предпочёл.
Однако, с точки зрения читаемости, всё сильно ухудшилось. Метод GetProductDetails удлиннился с пяти до одиннадцати строчек кода. Защитное программирование более чем удвоило количество строк! Проход по телу метода тонет в защитных конструкциях, таким образом, метод стал читаться хуже. Если вы типичный программист, то вы читаете код значительно больше, чем пишете его, так что практика, которая делает ваш код более трудночитаемым, должна вызвать в вас ощущение запашка. Нет сомнений в том, что многие программисты считают защитное программирование плохой практикой.
Устойчивость.
Возможно ли сбалансировать факторы, влияющие на проблему и её решение? Да, возможно, но для того, чтобы понять как, вы должны понимать корневую причину проблемы. Первоначальный пример кода не особенно сложен, но даже несмотря на это, присутствует множество случаев, когда этот код провалиться. Что касается нулевых ссылок, то причиной является ошибка в дизайне языка, но в целом, вопрос заключается в том, можете ли вы доверять входным данным или нет. Возвращаемые значения через вызов IUserRepository.Get это (косвенно) тоже входные данные.
В зависимости от того в какой среде функционирует ваша программа, у вас либо есть возможность доверять входным данным, либо у вас такой возможности нет. Представим, на минуту, ситуацию, при которой ваше ПО эксплуатируется в «дикой местности». Вашим ПО может быть повторно используемая библиотека или фреймворк. В таком случае, вы вообще не можете доверять входным данным. Если это так, то, возможно, вы захотите применить принцип устойчивости и оставаться убеждённым в том, что вы действуете в защитном стиле не только относительно входных, но и выходных данных. Другими словами, вы не хотите передавать нулевые ссылки (или другие дьявольские значения) и другому взаимодействующему коду.
Пример кода, приведённый ранее, может передавать нулевые ссылки своим зависимостям, например, если userId = 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, то баг находится в этом классе, не в потребителе этого класса. С учётом следования этому правилу, предыдущий код может быть сокращён до такого:
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. Этот принцип также известен под другим названием: инкапсуляция.
Заключение.
Как всегда, помогает понимание факторов, лежащих в основе проблемы или понимание вашей кодовой базы. Вам следует писать значительно больше защитных конструкций в случае, если ваш код живёт в «дикой местности», чем тогда, когда он живёт в защищённой среде. По сей день, я считаю заблуждением веру в то, что вы можете обойтись написанием неряшливого кода; мы все должны быть программистами-рэйнджерами.
Бесструктурный защитный код вредит читаемости. Защитный код – это всего лишь ещё одна отговорка для написания спагетти-кода. Напротив, структурированное защитное программирование являет собой инкапсуляцию. Я знаю, что мне предпочесть.
Автор: EngineerSpock
