Некоторое время назад в блоге был пост Наткнулся на образчик кода из категории "Да не дай боже такое сопровождать!". Я там прошелся по качеству кода от одного из RSDN-овских форумчан. Код, как по мне, из категории "лучше бы такого никогда не видеть, тем более не сопровождать".
А на днях этот же форумчанин открыл на RSDN-е новую тему, от которой я испытал что-то вроде шока. Почему шока постараюсь объяснить ниже (хотя не уверен, что это получится).
Итак, суть в том, что у человека образовалась цепочка из if-else в количестве 200 (двухсот(!!!)) штук. Из-за чего VC++ отказался компилировать данную развесистую конструкцию с ошибкой "MSVC C1061: compiler limit: blocks nested too deeply". И автор топика спрашивает у читателей что можно сделать в такой ситуации.
В одном из своих ответов в начатом обсуждении автор темы привел пример того, с чем он имеет дело:
|
else if ( opt.setParam("VAR:VAL") || opt.isOption("set-var") || opt.isOption("set-condition-var") || opt.isOption('C') || opt.setDescription("Set variable valie for conditions and substitutions")) { if (argsParser.hasHelpOption) return 0; if (!opt.hasArg()) { LOG_ERR<<"Setting condition variable requires argument (--set-condition-var)\n"; return -1; } auto optArg = opt.optArg; if (!appConfig.addConditionVar(optArg)) { LOG_ERR<<"Setting condition variable failed, invalid argument: '" << optArg << "' (--set-condition-var)\n"; return -1; } return 0; } |
При этом автор заявляет буквально следующее:
Обработка ключей командной строки.
На какой-нибудь map с хэндлерами не переделать, потому что у меня в режиме --help пробегается по этим условиям и собирает выдаваемую потом информацию. Тут в if сразу и длинный ключ set-var задаётся, и короткий C, и подсказка по формату значения VAR:VAL и описание опции Set variable valie for conditions and substitutions — в одном месте всё задаётся
Это, собственно, и подвигло меня написать данный пост. Т.е. сперва я был шокирован самим фактом появления данной темы на RSDN: это же сделал профессиональный программист, которому деньги платят за написание кода. Написание нормального кода. Т.е. задачей программиста изначально является то, чтобы такого говна в коде не было. Вот просто от слова совсем. И если ты этого обеспечить не можешь, то может нужно профессию сменить? В менеджмент уйти, к пример. Ну или подучиться немного этой самой профессии...
Но вишенкой на торте стало это самое "На какой-нибудь map с хэндлерами не переделать". Тут уж мое терпение лопнуло и решился таки пожертвовать частью своего выходного дня, чтобы написать данный текст.
Я даже не буду затрагивать тему того, что в продвинутых инструментах для работы с аргументами командной строки многие проблемы уже решены "by design". Например, можно посмотреть на Lyra или args, или даже boost::program_options. Мало ли какие условия заставили человека использовать что-то специфическое (хотя, подозреваю, что им же и написанное). Поэтому сконцентрируемся только на длинной цепочке if-else.
В общем, поскольку в нашем распоряжении только небольшой кусочек демонстрационного кода, то будем отталкиваться именно от него. Из-за чего предлагаемые ниже решения, скорее всего, не подойдут на 100% автору RSDN-новского топика, т.к. наверняка есть какие-то неозвученные на RSDN-е моменты. Но и нет цели показать 100% решение, смысл в том, чтобы показать тот вариант, который лично мне сразу же пришел в голову. И от которого можно оттолкнуться, чтобы избежать длинной портянки из if-else.
Суть в том, что с каждым if-ом у нас есть два связанных блока кода. Первый отвечает за описание одного аргумента командной строки (он помещается внутри условия в if-е). Второй отвечает за обработку аргумента (он помещается внутри then-ветки для if-а).
А раз так, то пусть каждый из блоков представляется в виде std::function. И оба эти блока пусть объединяются в рамках одной структуры:
|
using arg_preparation_block = std::function<bool(Option & opt)>; using arg_handling_block = std::function<int()>; struct arg_processing_info { arg_preparation_block _preparation; arg_handling_block _handling; }; |
Имея структуру arg_processing_info мы можем сформировать контейнер с описаниями всех нужных нам опций:
|
ArgParser argParser; AppConfig appConfig; ... // Еще что-то, что нужно в процессе обработки опций. std::vector< arg_processing_info > args{ { ._preparation = [&](Option & opt) -> bool { return opt.setParam("VAR:VAL") || opt.isOption("set-var") || opt.isOption("set-condition-var") || opt.isOption('C') || opt.setDescription("Set variable valie for conditions and substitutions") ; }, ._handling = [&]() -> int { if (argsParser.hasHelpOption) return 0; if (!opt.hasArg()) { LOG_ERR<<"Setting condition variable requires argument (--set-condition-var)\n"; return -1; } auto optArg = opt.optArg; if (!appConfig.addConditionVar(optArg)) { LOG_ERR<<"Setting condition variable failed, invalid argument: '" << optArg << "' (--set-condition-var)\n"; return -1; } return 0; } }, ... // Другие опции. }; |
А по этому контейнеру можно будет пробежаться обычным циклом:
|
Option opt; for(auto & info : args) { if(info._preparation(opt)) { // Похоже, что была выбрана наша опция. return info._handling(); } } |
Конечно, при этом мы все равно останемся в рамках большой по объему функции, в которой будет описываться куча опций и это описание будет занимать сотни строк. Так что у нас на руках окажется что-то дурно пахнущее. Но, по крайней мере, большую часть этой функции будут занимать декларативные описания, что гораздо лучше, чем портянка из if-else.
Но я бы пошел еще дальше. Есть у меня подозрение, что разбор аргументов командной строки можно было бы вынести в отдельный класс, который бы хранил в себе ArgParser и прочие вспомогательные объекты. И этот класс можно было бы сделать чем-то вроде вот такого:
|
Option opt; for(auto & info : args) { if(info._preparation(opt)) { // Похоже, что была выбрана наша опция. return info._handling(); } } |
В этом случае у нас нигде не будет длинных портянок кода. Да и сам код, как по мне, будет более декларативным и поддерживать его должно быть проще.
Есть у меня специфическая профдеформация: я не делаю программные продукты для конечного пользователя, результатом моего труда является код, который затем используется другими программистами в решении их задач.
Прикладной программист может оперировать категориями вроде "сделал что-то, что снизило время ожидания списка ближайших банкоматов на 20%" или "сделал что-то, что снизило расходы на облачную инфраструктуру на 12.8%". В результатах моего труда нет подобного прямого выхлопа. Все, что я делаю -- это даю другим людям возможность более просто достигать этих самых процентов.
В связи с этим у меня несколько "сбитые" оценки качества кода. Я оцениваю код не столько в показателях видимых конечному пользователю (тех самых 20% сокращения времени ожидания или 12.8% уменьшения стоимости), сколько в более специфических понятиях, таких как "понятность", "сопровождаемость", "надежность", "простота расширения", "сложность неправильного использования" и пр. штуки, которые не так-то просто и объяснить.
Бывает, что будучи консультантом захожу в проект и вижу там код, который не могу назвать хоть сколько-нибудь нормальным. Но он работает, приносит заказчику прибыль и авторы этого кода свою работу таки сделали. Однако, лично я часто задаюсь вопросом: а насколько бы было быстрее и дешевле, если бы код был написан нормально. Просто нормально, а не в виде говнокода с функциями по 700 строк или портянками на 200 if-else... И нутром чую, что было бы и быстрее, и дешевле. Но математически выразить не могу 🙁
С некоторых пор при общении на профильных форумах у меня появилась привычка спрашивать у собеседника где я могу посмотреть на написанный им код. Ибо на словах легко быть гуру от программирования. Но когда на практике оказывается, что человек пишет код как курица лапой, то просто жалко потраченного на общение с ним времени.
Если кто-то захочет поинтересоваться качеством моего кода, то без проблем -- его легко найти на GitHub-е. Я тут многократно рассказывал и про SObjectizer, и про RESTinio. Так что найти не сложно, но там результат работы моей команды. Если хочется посмотреть на код, который написан мной практически полностью в одиночку, то можно заглянуть в потроха библиотеки timertt (описание можно найти здесь). Только сразу дисклеймер: на звание хорошего программиста даже и не претендую, максимум -- это чуть выше среднего, да и это разве что в лучшем случае. Посему гнилыми помидорами прошу не бросаться -- программирую как умею. А если вы увидели в коде косяки и непростительные корявства -- то поделитесь, плиз, способами улучшить. Буду очень признателен. В конце-концов, этим кодом пользуются люди, так что ваши предложения помогут не только мне.
Комментариев нет:
Отправить комментарий