Не так давно сидел я делал ревью кода одного из коллег. Это было не первое мое ревью, но в этот раз я задался вопросом как все таки формализовать подход и на что конкретно стоит обращать внимание и как аргументировать и формулировать предложения и замечания. Сформулировал я для себя вот такие пункты:
Соблюдение принципов SOLID
На всякий случай напомню, SOLID - это аббревиатура для 5 основных принципов ООП:
Single Responsibility Principle - принцип единой ответственности. Этот принцип говорит, что для класса должна быть определена единственная ответственность. Или еще его иногда формулируют как “для внесения изменений в класс должна быть только одна причина”
Open Closed Principle - принцип открытости-закрытости. Этот принцип говорит, что программные сущности должны быть открыты для расширения, но закрыты для модификации.
Liskov Substitution Principle - принцип подстановки Барбары Лисков. Роберт Мартин формулирует этот принцип так: “Функции, которые используют базовый тип, должны иметь возможность использовать подтипы базового типа, не зная об этом.”
Interface Segregation Principle - принцип разделения интерфейсов. Этот принцип говорит, что “много интерфейсов специального назначения лучше, чем один интерфейс общего назначения”.
Dependency Inversion Principle - принциц инверсии зависимостей. Этот принцип говорит, что абстракции не должны зависеть от реализаций, наоборот, реализации должны зависеть от абстракций.
Отсутствие дурных запахов
Обычно выделяют около 20 запахов и в идеале бы, конечно, знать и отслеживать их все. Их все я здесь перечислять не буду, но вот наиболее часто встречающиеся и сильнее других бросающиеся в глаза:
Его величество дублирование кода
Магические числа
Операторы типа switch
Длинный метод
Длинный список параметров
Неинформативные имена переменных
Дурные запахи (не все) и принципы SOLID взаимосвязаны и иногда наличие запаха может сигнализировать о нарушении одного из принципов, к примеру дублирование кода может говорить о нарушении принципа единой ответственности. Однако, на мой взгляд все равно стоит оставлять комменты и о запахе и о нарушении.
Правильное использование языка
Сюда я отнес использование условных языковых идиом, хороший практик и рекомендаций из документации. Приведу пару примеров. Я пишу на питоне, так что примеры тоже будут на питоне.
Использовать литеральную форму задания словаря, вместо циклов, когда это возможно:
chars = ['a', 'b', 'c']
# Bad
d = {}
for i in range(len(chars)):
d[i] = chars[i]
# Good
d = {i: char for i, char in enumerate(chars)}Пример выше нужен лишь для иллюстрации, он искусственный и создать такой словарь можно конечно же по другому
Использовать подходящие методы встроенный типов:
colors_codes = {
'red': '#FF0000',
'green': '#008000'
}
white_name = 'white'
white_code = '#FFFFFF'
# Bad
if white_name not in colors_codes:
colors_codes[white_name] = white_code
print(colors_codes[white_name])
# Good
print(colors_codes.setdefault(white_name, white_code))Использовать менеджеры контекста:
#Bad
...
fin = open(path, 'rt')
text = fin.read()
print(text)
...
# Good
...
with open(path) as fin:
text = fin.read()
print(text)
...и т.д.
Покрытие кода тестами
В этом пункте проверяю не количество тестов, а их качество: есть ли тесты для граничных случаев и есть ли непокрытые кейсы.
Конечно, есть и другие моменты, о которых можно задуматься: архитектура и вопросы производительности, к примеру, но их я еще не формализовал.
SOLID принципы конечно же не для всех случаев актуальны, да и с некоторыми запахами в определенных ситуациях можно не бороться. Пункты выше относятся к проектам, где они имеют смысл.
