Comments 9
В данном примере ревьюер предложил улучшения, которые касаются обработки
граничных случаев о чем не предусмотрел разработчик, эти изменения
сделали код более эффективным.
Это изменение не дает ничего, код и так вернет 0 для n < 1
. А вот проверки на переполнение тут нет, и ревьювер это пропустил. От такого ревью пользы никакой
1. Изменение, предложенное ревьюером, касающееся проверки на n <= 0
, имеет смысл. Оно делает код более явным и понятным. Вместо того чтобы полагаться на неявное поведение цикла (который не выполнит ни одной итерации для n < 1
), мы явно обрабатываем случай n <= 0
и возвращаем 0. Это улучшает читаемость и делает намерения автора кода более понятными для других разработчиков.
По мне, так оно делает код не более понятным, а более запутанным. Нет ничего страшного в том, чтоб полагаться на стандартное и очевидное поведение цикла, а вот добавлять масло маслянное во-первых раздувает код, во-вторых путает, ибо мозг при прросмотре кода на этом спотыкается, останавливается и начинает думать: "но зачем???"
Такая проверка имеет смысл исключительно тогда, когда для n меньше нуля нам надо выдать другое поведение, например, ошибку.
неявное поведение цикла
С чего это оно "неявное"? Это же база, так данный цикл работает во всех языках программирования. Ревьювил наверное джун, раз предложил такое.
Мне кажется, что надежная имплементация должна выбрасывать ошибку при задании отрицательного значения параметра.
func sumOfSquares(n int) (int, error) {
result := 0
var err error = nil
if n < 0 {
err = errors.New("sumOfSquares: negative argument")
} else if n != 0 {
for i := 1; i <= n; i++ {
result += i * i
}
}
return result, err
}
func main() {
sum, err := sumOfSquares(-1)
if err != nil {
// Complain ...
}
}
Важно чтобы код ревью проводил человек, которому не все равно, и который обладает достаточной компетенцией. Иначе никакие гайды не помогут
Можно микрооптимизировать ):
…
result := 1
for i := 2; i <= n; i++ {
result += i * i
}
…
Зачем нужен код-ревью?