Слышал такое выражение, что для того чтобы стать программистом, нужно быть ленивым. Но иногда лень в программировании приводит к возникновению ужасного технического долга. В своей заметке об SRP я упоминал, что нарушение этого принципа может привести к увеличению сложности или даже к умножению ее. Один из моих коллег произвел на свет интересный пример, и я решил с его помощью продемонстрировать как это выглядит.
Давайте определимся, что же такое эта избыточная сложность. Но для начала поговорим о ее противоположности, о сложности исходящей от требований. Для примера есть требования посчитать зарплату сотрудника из почасовой ставки и отработанных часов. И, если сотрудник работает в компании больше пяти лет, начислить бонус. Это «если» исходит из требований, и избежать его нельзя. В той или иной форме оно станет элементом сложности в коде приложения, скорее всего в виде условного оператора «if». Но иногда сложность не исходит из требований, а вытекает из подхода разработчика к решению задачи.
Оператор «if», паттерны такие как «стратегия», полиморфные методы — не полный список приемов программирования способных содержать эту избыточную сложность. Лично я, к слову, всегда против применения разработчиками паттернов просто потому что они это могут, а не для решения конкретной проблемы.
Вот простой пример. Он может показаться выдуманным, но это не так. Он даже не упрощен, именно в таком виде я встретил его во время code review пару лет назад. В двух местах в коде были вызовы одной и той же функции но с разным булевым параметром:
Подобные конструкции всегда выглядят подозрительно и эта функция меня не разочаровала. Этот параметр передавался с единственной целью, чтобы внутри этой функции быть проверенным:
Эту проверку можно описать как «если меня вызвали из места А, делаем одно, иначе меня вызвали из места Б, делаем другое». Этот флаг, этот «if» и есть то, о чем вся эта заметка. Сложность не исходящая от бизнес-требований. Естественно, я порекомендовал изменить код следующим образом:
Всё, избыточной сложности больше нет. Именно тут разработчик должен не полениться и написать еще одну сигнатуру функции.
Здесь можно воскликнуть: «Но это же всего один 'if'», или: «Это нарушение очевидно, кто вообще так код пишет?». И тут на сцену выходит второй пример. В нём видно, что увидеть нарушение может быть заметно сложнее, а также что стоимость этого нарушения может составлять больше чем всего один «if». Как и в первом примере, функция используется в двух местах:
Метод, как следует из его имени, проверяет валидность объекта. Однако было неочевидно, что он также может проверять валидность массива объектов. Я подправил имена переменных, чтобы сделать акцент на этом нарушении. Метод выглядит вот так:
Вот оно. Один «if» становится множеством «if»-ов. Если массив содержит 100 объектов, то этот «if» выполнится 101 раз. А на реальных данных у нас там могло быть 30 тысяч объектов, и это уже внушительный урон производительности.
Очевидно, следуя принципу единственной ответственности, этот метод нужно порефакторить так, чтобы получилось 2 метода:
Соотвественно также надо поправить точки вызова.
Занятно, что примеры, которые я приводил в заметке про SRP, вели к увеличению SLOC, эти же примеры, наоборот, ведут к небольшому его уменьшению, попутно с ожидаемым улучшением качества кода.
На этом всё. Всего лишь пара простых примеров для демонстрации важнейшего из принципов хорошего кода.
Давайте определимся, что же такое эта избыточная сложность. Но для начала поговорим о ее противоположности, о сложности исходящей от требований. Для примера есть требования посчитать зарплату сотрудника из почасовой ставки и отработанных часов. И, если сотрудник работает в компании больше пяти лет, начислить бонус. Это «если» исходит из требований, и избежать его нельзя. В той или иной форме оно станет элементом сложности в коде приложения, скорее всего в виде условного оператора «if». Но иногда сложность не исходит из требований, а вытекает из подхода разработчика к решению задачи.
Оператор «if», паттерны такие как «стратегия», полиморфные методы — не полный список приемов программирования способных содержать эту избыточную сложность. Лично я, к слову, всегда против применения разработчиками паттернов просто потому что они это могут, а не для решения конкретной проблемы.
Вот простой пример. Он может показаться выдуманным, но это не так. Он даже не упрощен, именно в таком виде я встретил его во время code review пару лет назад. В двух местах в коде были вызовы одной и той же функции но с разным булевым параметром:
// first place
doSomething(true);
// second place
doSomething(false);
Подобные конструкции всегда выглядят подозрительно и эта функция меня не разочаровала. Этот параметр передавался с единственной целью, чтобы внутри этой функции быть проверенным:
doSomething(flag: boolean): void {
if(flag) {
// do first thing
} else {
// do second thing
}
}
Эту проверку можно описать как «если меня вызвали из места А, делаем одно, иначе меня вызвали из места Б, делаем другое». Этот флаг, этот «if» и есть то, о чем вся эта заметка. Сложность не исходящая от бизнес-требований. Естественно, я порекомендовал изменить код следующим образом:
// first place
doFirstThing();
// second place
doSecondThing();
//method is split into 2 parts each having their own responsibility
doFirstThing(): void {
// do first thing
}
doSecondThing(): void {
// do second thing
}
Всё, избыточной сложности больше нет. Именно тут разработчик должен не полениться и написать еще одну сигнатуру функции.
Здесь можно воскликнуть: «Но это же всего один 'if'», или: «Это нарушение очевидно, кто вообще так код пишет?». И тут на сцену выходит второй пример. В нём видно, что увидеть нарушение может быть заметно сложнее, а также что стоимость этого нарушения может составлять больше чем всего один «if». Как и в первом примере, функция используется в двух местах:
// first place
checkValidity(obj);
// second place
checkValidity(arrayOfObjs);
Метод, как следует из его имени, проверяет валидность объекта. Однако было неочевидно, что он также может проверять валидность массива объектов. Я подправил имена переменных, чтобы сделать акцент на этом нарушении. Метод выглядит вот так:
checkValidity(parm: MyType | MyType[]): void {
if(Array.isArray(parm)) {
parm.forEach(p => checkValidity(p));
} else {
// here the object gets checked
// and conditional exception is thrown
}
}
Вот оно. Один «if» становится множеством «if»-ов. Если массив содержит 100 объектов, то этот «if» выполнится 101 раз. А на реальных данных у нас там могло быть 30 тысяч объектов, и это уже внушительный урон производительности.
Очевидно, следуя принципу единственной ответственности, этот метод нужно порефакторить так, чтобы получилось 2 метода:
checkItemsValidity(parms: MyType[]): void {
parms.forEach(p => checkItemValidity(p));
}
checkItemValidity(parm: MyType): void {
// here the object gets checked
// and conditional exception is thrown
}
Соотвественно также надо поправить точки вызова.
Занятно, что примеры, которые я приводил в заметке про SRP, вели к увеличению SLOC, эти же примеры, наоборот, ведут к небольшому его уменьшению, попутно с ожидаемым улучшением качества кода.
На этом всё. Всего лишь пара простых примеров для демонстрации важнейшего из принципов хорошего кода.