Несколько важных вопросов, которые следует задать при рассмотрении исправления ошибки.
Каждый разработчик регулярно выполняет анализ кода для исправления ошибок в рамках своих повседневных обязанностей. Процесс прост и понятен, но есть несколько передовых практик, которые разработчики должны помнить и следовать, чтобы разработать проект с хорошим качеством кода.
Как профессиональный разработчик, задаете ли вы себе эти вопросы, когда рассматриваете исправление ошибки вашего товарища по команде?
Есть ли изменения в модульном тесте?
Исправление ошибки в коде должно сопровождаться добавлением или изменением модульного теста.
Запрос на вытягивание, содержащий изменение кода для исправления ошибки, но не содержащий изменений в модульных тестах, - это запах.
Если изменение кода было внесено для исправления ошибки, но ни один из модульных тестов не был добавлен или изменен, то охват модульным тестом недостаточен.
Представьте, что разработчики реализовали метод преобразования некоторых строковых параметров в числа и покрыли его модульным тестом.
public int Convert(string option)
{
if (option == "No") return 0;
else if (option == "Yes") return 1;
else return -1;
}
[InlineData("No", 0)]
[InlineData("Yes", 1)]
[InlineData("Invalid", -1)]
public void VerifyConvert(string option, int expectedValue)
{
var actualResult = Convert(option);
Assert.Equal(expectedValue, actualResult);
}
Текущий модульный тест полностью охватывает метод Convert - уровень покрытия линий и ответвлений составляет 100%.
Представьте, что разработчик забыл добавить третий вариант «Может быть», и инженер по контролю качества сообщил об ошибке. Если разработчик устраняет проблему, просто добавляя отсутствующий параметр к методу Convert, не добавляя новый тестовый пример в модульный тест, метод больше не будет охватывать полное модульное тестирование.
public int Convert(string option)
{
if (option == "No") return 0;
else if (option == "Yes") return 1;
else if (option == "Maybe") return 2;
else return "-1";
}
[InlineData("No", 0)]
[InlineData("Yes", 1)]
[InlineData("Invalid", -1)]
//The test case for "Maybe" option is missing
public void VerifyConvert(string option, int expectedValue)
{
var actualResult = Convert(option);
Assert.Equal(expectedValue, actualResult);
}
Отсутствие тестового примера означает, что если какой-либо разработчик позже случайно изменит «2» на «20» или вообще удалит параметр «Может быть» во время рефакторинга, модульный тест все равно будет зеленым, потому что тестовый пример [InlineDate("Maybe", 2)] не был реализован с новым опция добавлена к методу. Проблема будет обнаружена через некоторое время, возможно, даже в производственном коде. Следовательно, для любого исправления в бизнес-логике должны быть изменения в соответствующих модульных тестах. Обзор кода - идеальное время, чтобы обнаружить отсутствие модульного теста.
Мы устраняем основную причину или просто симптом?
Важно понимать, пытается ли разработчик устранить основную причину ошибки или просто ее симптом. Поиск первопричины часто занимает больше времени, чем просто устранение симптома. Но это единственно правильный способ что-то сделать, и давайте разберемся, почему:
public decimal CalculateOrderPrice(int orderId)
{
Order order = _orderRepository.GetOrder(orderId);
decimal price = order.Price; //Potential null reference here
}
Класс Order является ссылочным типом, он может содержать нулевое значение, и приложение выйдет из строя при попытке доступа к свойству Price.
Самый простой способ «исправить» проблему - добавить нулевую проверку и забыть о ней. Однако исключение нулевой ссылки - это всего лишь симптом более серьезной проблемы, такой как логика создания заказа. Эта исходная основная причина может вызывать проблемы в разных местах приложения, и простое добавление нулевой проверки не решит основную проблему.
Разработчики всегда должны находить и устранять основную причину проблемы, чтобы полностью устранить все ее симптомы. Если у разработчиков есть время только для устранения симптома из-за давления со стороны бизнеса или по другой причине, им все равно необходимо определить основную причину и создать тикет о техническом долге в отставании по продукту.
Последние мысли
Есть много других вопросов, которые могут возникнуть во время проверки кода, в зависимости от вашего проекта. Если в вашем проекте нет сквозных автоматизированных тестов, вы можете спросить: «Описал ли разработчик область воздействия изменения для инженеров контроля качества?». Если в вашем проекте есть комплексные технические решения. документации, вы можете спросить: «Была ли обновлена документация, чтобы отразить изменения в исходном коде?».
Всегда задавайте вопросы. Вы можете начать с тех, которые я описал в статье, а затем расширить список своими собственными, чтобы лучше выполнять обзор кода.
