Pull to refresh

Comments 9

В данном примере ревьюер предложил улучшения, которые касаются обработки
граничных случаев о чем не предусмотрел разработчик, эти изменения
сделали код более эффективным.

Это изменение не дает ничего, код и так вернет 0 для n < 1. А вот проверки на переполнение тут нет, и ревьювер это пропустил. От такого ревью пользы никакой

1. Изменение, предложенное ревьюером, касающееся проверки на n <= 0, имеет смысл. Оно делает код более явным и понятным. Вместо того чтобы полагаться на неявное поведение цикла (который не выполнит ни одной итерации для n < 1), мы явно обрабатываем случай n <= 0 и возвращаем 0. Это улучшает читаемость и делает намерения автора кода более понятными для других разработчиков.

По мне, так оно делает код не более понятным, а более запутанным. Нет ничего страшного в том, чтоб полагаться на стандартное и очевидное поведение цикла, а вот добавлять масло маслянное во-первых раздувает код, во-вторых путает, ибо мозг при прросмотре кода на этом спотыкается, останавливается и начинает думать: "но зачем???"

Такая проверка имеет смысл исключительно тогда, когда для n меньше нуля нам надо выдать другое поведение, например, ошибку.

неявное поведение цикла

С чего это оно "неявное"? Это же база, так данный цикл работает во всех языках программирования. Ревьювил наверное джун, раз предложил такое.

Такое было бы осмысленно, если бы вместо цикла из 2 строчек была простыня на 2 экрана с ifами. Тогда лучше явно изобразить early return.

Мне кажется, что надежная имплементация должна выбрасывать ошибку при задании отрицательного значения параметра.

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
}
…

Sign up to leave a comment.

Articles