# Статический анализатор подталкивает писать чистый код Статические анализаторы помогают не только обнаруживать ошибки и дефекты безопасности, но и делать код чище\. Выявляя лишние проверки, дублирующие действия и другие аномалии, можно сделать код проще, красивее и легче для чтения\. Разберём это на практическом примере рефакторинга функции\. ![1115_clean_code_ru/image1.png](https://import.viva64.com/docx/blog/1115_clean_code_ru/image1.png) Сразу перейдём к делу и начнём разбирать фрагмент кода на языке C, взятый из проекта iSulad\. ```cpp /* container state to disk */ int container_state_to_disk(const container_t *cont) { int ret = 0; if (cont == NULL) { return -1; } ret = container_save_container_state_config(cont); if (ret != 0) { return ret; } return ret; } ``` На первый взгляд вполне опрятный код\. Беда в том, что он избыточен и заставляет тратить на своё изучение больше времени, чем заслуживает\. Я наткнулся на этот фрагмент кода благодаря предупреждению анализатора PVS\-Studio: [V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the subsequent code fragment\. container\_unix\.c 878 Речь идёт об этом месте: ```cpp if (ret != 0) { return ret; } return ret; ``` Действительно, вне зависимости от того, какое значение имеет переменная _ret_, выполняется одно и то же действие\. Это не баг\. Однако предупреждение анализатора заставляет задуматься о чистоте кода и, как вы сейчас увидите, "потянув за верёвочку", можно провести хороший рефакторинг\. Для начала сократим нижнюю часть кода до одного _return_: ```cpp /* container state to disk */ int container_state_to_disk(const container_t *cont) { int ret = 0; if (cont == NULL) { return -1; } ret = container_save_container_state_config(cont); return ret; } ``` Давайте не будем останавливаться на этом и продолжим улучшать код\. Объявлять переменные в начале функции — это устаревший, вредный стиль написания кода\. Он берёт своё начало от языков, где в начале функции требовалось объявлять все переменные\. Но в их предварительном объявлении нет ничего замечательного, а наоборот, увеличивается шанс допустить ошибку\. Например, начать использовать ещё неинициализированную переменную\. Рекомендуется объявлять переменную как можно ближе к месту её использования\. Более того, лучше сразу её инициализировать в точке объявления\. Здесь переменная _ret_ тоже инициализируется при объявлении\. Но это "не та инициализация", она просто не имеет смысла\. В теории, она может защитить от ошибки использования неинициализированных переменных\. Однако лучше писать так, чтобы в принципе такой возможности не было\. Для этого как раз и рекомендуют совместить объявление переменной с её инициализацией полезным значением\. В нашем случае это выглядит так: ```cpp /* container state to disk */ int container_state_to_disk(const container_t *cont) { if (cont == NULL) { return -1; } int ret = container_save_container_state_config(cont); return ret; } ``` При этом становится очевидно, что переменная _ret_ вообще не нужна\. Это повод избавиться от неё: ```cpp /* container state to disk */ int container_state_to_disk(const container_t *cont) { if (cont == NULL) { return -1; } return container_save_container_state_config(cont); } ``` Получилось упростить и сократить код\. Good\. Здесь можно возразить, что предыдущий вариант кода был написан из расчёта его дальнейшего расширения\. Ой, бросьте\. Не надо писать код про запас\. С большой вероятностью это расширение никогда не понадобится\! А если такое произойдёт, то ничего сложного в добавлении кода нет\. Здесь уместна эта картинка: ![1115_clean_code_ru/image2.png](https://import.viva64.com/docx/blog/1115_clean_code_ru/image2.png) Продолжим\. Обратите внимание на комментарий к функции\. В нём нет никакого смысла\. Это калька с названия функции: ```cpp /* container state to disk */ int container_state_to_disk(const container_t *cont) ``` Комментарий ничего не поясняет, ничем не помогает\. Это бессмысленная сущность, которую следует удалить\. ```cpp int container_state_to_disk(const container_t *cont) { if (cont == NULL) { return -1; } return container_save_container_state_config(cont); } ``` Перед нами простая, красивая функция\-прослойка, которая проверяет указатель и передаёт управление другой функции\. На этом всё? Оказывается, нет\. Заглянем в ту — другую — вызываемую функцию: ```cpp static int container_save_container_state_config(const container_t *cont) { int ret = 0; parser_error err = NULL; char *json_container_state = NULL; if (cont == NULL) { return -1; } container_state_lock(cont->state); .... } ``` Ха\! Оказывается, эта функция и сама имеет в себе проверку указателя, и ей можно безопасно передать _NULL_\. Соответственно код упрощается ещё больше: ```cpp int container_state_to_disk(const container_t *cont) { return container_save_container_state_config(cont); } ``` Что это значит? Что две эти функции — синонимы\. Это стоило написать в комментарии и не создавать бессмысленные проверки и действия в функции\. Как мы выяснили, всё это был мусорный код, который ничего не делает и только занимает место\. Чего не хватает, так это приблизительно такого комментария: ```cpp /* На данный момент функция container_state_to_disk является синонимом container_save_container_state_config. Это изменится, когда будет реализовываться функционал ....... */ int container_state_to_disk(const container_t *cont) { return container_save_container_state_config(cont); } ``` А если нет никакого "когда будет реализовываться"? Тогда текст программы можно упростить и сократить ещё больше с помощью вот такого макроса: ```cpp #define container_state_to_disk container_save_container_state_config ``` К этому макросу даже комментарий не нужен\. И так понятно, что два эти названия являются синонимами\. P\.S\. Вообще, я [не очень люблю макросы](https://habr.com/ru/companies/pvs-studio/articles/444612/)\. Но без них в C жить тяжело, и аккуратное их использование не портит и не усложняет код\. Поэтому я посчитал вполне уместным всё в итоге свести к макросу\. Пишите простой код\. Не пишите избыточный код про запас\. Спасибо за внимание\.