[Из песочницы] Практика рефакторинга. Завистливые функции
Однажды наша команда обнаружила, что сильно проседает производительность системы при выполнении довольно простого SQL запроса:
select count (*) n from products where category_id = ? Разумеется, встал вопрос, как его оптимизировать.
Подкованный читатель, возможно, сразу подумает об индексах, хинтах и прочих фичах СУБД. Но сегодня рассказ будет не о них. Да и вообще, не затронет тему оптимизации SQL запросов.
Сегодня речь пойдёт об очень простом методе рефакторинга, который в данном конкретном случае позволил значительно увеличить производительность системы.
Этот запрос находился в том старом коде, в который уже пару лет никто не лазил, в классе SQLConsts среди прочих других SQL запросов:
public class SQLConsts { public static final String PRODUCTS_SQL = «select count (*) n from products where category_id = ?»; … А использовался он в другом классе — ProductProcessor:
public class ProductProcessor { … private boolean isCategoryVisible (int categoryID) { ResultSet resultSet = executeQuery (SQLConsts.PRODUCTS_SQL, categoryID); int n = resultSet.getIntegerFieldByName («n»); return n > 0; } … Даже не очень опытный программист наверняка заметит, что излишне вычислять в запросе количество строк, если потом это количество просто сравнивается с нулём.
Как же появился этот очевидный эпикфейл? Анализ Git-логов показал, что изначально в методе isCategoryVisible была более сложная логика, которая использовала количество строк в своих вычислениях. Но потом от сложной логики отказались, и осталось только n > 0. Видимо, у того программиста, который делал эти изменения, просто не возник вопрос, что же такое n, и он не стал смотреть сам SQL запрос, тем более, что тот находился совсем в другом файле.
Теперь, когда эти два куска кода находятся рядом, оптимизация становится очевидной. В итоге метод isCategoryVisible был переписан: select count (*) заменили на конструкцию с where exists, что дало ощутимый прирост производительности на больших объёмах данных;, а от класса SQLConsts впоследствии избавились.
public class ProductProcessor { … private boolean isCategoryVisible (int categoryID) { ResultSet resultSet = executeQuery ( «select null from dual where exists (select null from products where category_id = ?)», categoryID ); return! resultSet.isEmpty (); } … Отсюда правило: «то, что изменяется одновременно, надо хранить в одном месте. Данные и функции, использующие эти данные, обычно изменяются вместе», — писал Мартин Фаулер в своей книге «Рефакторинг. Улучшение существующего кода» более десяти лет назад.
В нашем случае данные (SQL запрос) хранились в одном классе — SQLConsts, а функция isCategoryVisible, которая использовала эти данные — в другом: в ProductProcessor. Такие функции Фаулер называет завистливыми, потому что они больше интересуются не тем классом, в котором находятся, а каким-то другим. И чаще всего предметом зависти являются данные.
Еще раз: то, что изменяется одновременно, надо хранить в одном месте — повторяйте это как мантру, пока это правило не войдёт у вас в привычку. Когда вы уже перестанете о нём думать и будете следовать ему на подсознательном уровне, вы сами не заметите, как ваш код станет чище.
P. S. А что, если бы переменная n называлась, к примеру, productCount, а константа PRODUCTS_SQL — PRODUCT_COUNT_IN_CATEGORY? Тогда productCount > 0 подтолкнуло бы разработчика задуматься, нужно ли ему вычислять в запросе количество.Поэтому второе правило: давайте понятные имена переменным, константам, методам и классам. Возможно, это правило даже важнее правила первого.
