Поддержан механизм обязательных параметров и их проверки #14
Conversation
… `ОбязательныеПараметрыЗаполнены` Обновлён модуль тестирования Обновление тестов
src/cmdline.os
Outdated
| Функция ДобавитьПараметр(Знач ИмяПараметра, Знач Пояснение = "") Экспорт | ||
|
|
||
| Лог.Отладка("ДобавитьПараметр: ИмяПараметра <"+ИмяПараметра+">"); | ||
| Лог.Отладка(СтрШаблон("ДобавитьПараметр: ИмяПараметра <%1>", ИмяПараметра)); |
There was a problem hiding this comment.
операции логера внутри вызывают стршаблон.
Можно использовать Лог.Отладка("ДобавитьПараметр: ИмяПараметра <%1>", ИмяПараметра)
src/cmdline.os
Outdated
| КонецФункции | ||
|
|
||
| Функция ДобавитьПараметрВТаблицу(Знач Таблица, Знач Имя, Знач Пояснение, Знач Флаг, Знач Глобальный = Ложь) | ||
| Функция ДобавитьПараметрВТаблицу(Знач Таблица, Имя, Пояснение, Флаг, Глобальный, Обязательный) |
There was a problem hiding this comment.
ИМХО зря отказался от параметра по умолчанию для Знач Глобальный = Ложь
я посмотрел использование ДобавитьПараметрВТаблицу в коде
сейчас при его вызова все равно явно указывается 2 последних параметра как Ложь, Ложь, что в итоге затрудняет чтение.
Предлагаю сделать оба параметра Глобальный, Обязательный по умолчанию ложными и упростить код аналогично тому, что было.
There was a problem hiding this comment.
++ Это ошибка, при тестировании через вс код потерялись некоторые изменения и при восстановлении потерялось. Спс.
| // Возвращаемое значение: | ||
| // Булево - Если заполнены все параметры, то возвращает Истина, в противном случае Ложь | ||
| // | ||
| Функция ОбязательныеПараметрыЗаполнены(ВыводитьОшибки = Ложь) Экспорт |
There was a problem hiding this comment.
1 Непонятно назначение и применение этого публичного метода.
сейчас он вызывается только в коде тестов, но не в боевом коде.
2 и я в коде не увидел другой проверки наличия обязательных параметров :(
т.е. выглядит все так, будто парсер сейчас не проверяет наличие обязательных параметров при разборе :(
клиент может указать, что параметр является обязательным, но проверять наличие такого параметра он должен сам?
ИМХО такие параметры долже отслеживать парсер и выдавать ошибку. это его задача, а не клиента.
зачем клиента нагружать этой проверкой??
There was a problem hiding this comment.
Это точно хорошая практика? В своё время я придержался мысли, что контроль поток за счёт исключений это не самая лучшая идея (антипатерн), http://wiki.c2.com/?DontUseExceptionsForFlowControl
На мой вкус Разобрать(..) должен возвращать Неопределено, если у него что-то там поломалось, но текущую логику для легаси ломать не стал.
Если есть уверенность (я нуб в части вашего подхода), что бибилотеки могут в любой момент упасть и все вызовы методов библиотека мы должны убирать в попытку..исключение, то ок, заверну так.
There was a problem hiding this comment.
Тут вот зачем исключения. Если пользователь неверно указал параметры, то программа работать не сможет. Поэтому бросается исключение. Оно или просто выведется или перехватится автором клиенсткого приложения и автор покажет красивую, например, справку. Т.е. DontUseExceptions - он немного не про это. В текущем кейсе выброс исключения оправдан.
There was a problem hiding this comment.
Т.е. я поддержал бы @artbear Не зачем нагружать клиента проверкой незаполненности. Если что-то обязательное незаполненное, то это в любом случае нештатная ситуация. Ее можно не перехватывать, текст исключения все равно несет указание на ошибочно незаполненный параметр. Можно это исключение перехватить и показать справку с помощью метода вывода справки cmdline. Но в любом случае, это аварийная остановка приложения.
|
|
||
| ВсеПараметрыЗаполнены = Ложь; | ||
|
|
||
| Если ВыводитьОшибки И Лог.Уровень() > 0 Тогда |
There was a problem hiding this comment.
1 проверка уровня логов через магическую константу 0 неверна, нужно оперировать спец.перечислением УровниОтладки.ХХХ
2 да и вообще зачем здесь хитрить с логами-то?
выдаем как Лог.Ошибка и все
3 как я уже писал, проверку обязательных должен делать парсер, и он должен выбрасывать ошибку=исключение
в итоге параметр метода ВыводитьОшибки совсем не нужен.
There was a problem hiding this comment.
кратко ВызватьИсключение СтрШаблон("Не заполнен обязательный параметр %1", Строка.Имя);
There was a problem hiding this comment.
жду ответ по #14 (comment)
и поправлю в соответствии с вашим подходом.
There was a problem hiding this comment.
кратко
ВызватьИсключение СтрШаблон("Не заполнен обязательный параметр %1", Строка.Имя);
мы тогда не получим ошибку по всем параметрам сразу, а будем спотыкаться о каждый следующий незаполненный параметр при запуске, а показать пользователю все его ошибки сразу из-за ВызватьИсключение у нас не выйдет, так как парсер падает при первой же проблеме
There was a problem hiding this comment.
Можно тогда НСтр("ru='Не заполнены следующие обязательные параметры: %1'") и выдать полный список незаполненных параметров.
There was a problem hiding this comment.
Стремление показать ошибки все сразу - хорошее. Но если честно, то я регулярно натыкаюсь в юникс утилитах на обратное поведение. Падает при каждой встреченной ошибке, мол "исправь тут", а потом на следующей и т.д...
Если возникает сложность с "показать сразу все", то я бы и не стал это реализовывать. Большинство консольных утилит себя так дружелюбно не ведет.
|
@nixel2007 Меня то за что! :))) Я уже давно пользуюсь только |
5 participants
No description provided.