Best Practice in real life: SysPolicy form as a bad example

[Blog bot]

Источник: http://alexvoy.blogspot.com/2019/11/...syspolicy.html
==============

Walk the talk, guys! It can save a life one beautiful day!

Happy weekend to everyone

Источник: http://alexvoy.blogspot.com/2019/11/...syspolicy.html

[Владимир Максимов]

Я что-то не понимаю пафоса. А где здесь криминал?

validateWrite() нужна в том случае, если нет уверенности в корректности заполнения полей. Обычно это происходит в случае, если поля заполняются пользователем. Он вполне может забыть что-то указать или указать не корректно

А здесь-то совсем другой случай. Все ключевые поля заполнены. Ну, и зачем "лишние" проверки?

Нет, я понимаю, что если выполняется кастомизация и в validateWrite() добавили некую проверку, которую ожидали при любом добавлении записи. То, да, такой код - это "сюрприз". Но, при чем здесь Best Practices? Это проблемы разработичка или консультанта/тестера. Смотря кто пропустил такую ситуацию. Вы же не настаиваете на выполнение validateField() для каждого поля, хотя там тоже много чего можно изменить. Или тоже необходимо?

[trud]

Цитата:

Сообщение от Владимир Максимов

А здесь-то совсем другой случай. Все ключевые поля заполнены. Ну, и зачем "лишние" проверки?

Откуда вы знаете что все ключевые поля заполнены? Для этого и нужен validateWrite, который собственно это проверит.
Я для такого обычно использую следующий метод

public static void validateWriteRecordCheck(Common _record)
    {
        if (! _record.validateWrite())
        {
            throw error(strFmt("Failed to write %1 table", tableId2pname(_record.TableId)));
        }
    }

https://github.com/TrudAX/XppTools/b...xClass/DEV.xml

[Владимир Максимов]

Цитата:

Сообщение от trud

Откуда вы знаете что все ключевые поля заполнены?

Именно потому, что это программное создание строки без участия пользователя. Т.е. здесь нет "внешних" данных, которые мог бы подготовить пользователь и в которых могла бы вкрасться ошибка. Все данные готовит разработчик. И, естественно, они должны быть корректными. У Вас это в релиз не пройдет, если что-то не заполнено. Ну, это все-равно, что проверять тип переданного в метод параметра, при том, что он указан явно в EDT. А вдруг не то передали?

Ну, представим, что добавили validateWrite() и он вернул false. Что в этой ситуации может сделать пользователь? Да ничего! Запись же программно создается. Получаем "мертвый" код, который вообще не может быть использован.

Нет, я понимаю, почему хочется поставить validateWrite(). Сам иногда это делаю. А вдруг?!. Но в данном конкретном случае - это явный over-programming. Не вижу никакого криминала, если в описанном сценарии этой проверки нет

[trud]

Цитата:

Сообщение от Владимир Максимов

Ну, представим, что добавили validateWrite() и он вернул false. Что в этой ситуации может сделать пользователь? Да ничего!

Почему ничего - он зарепортит ошибку. Ошибочная запись создана не будет. Могут кстати и не только добавить validateWrite, но и добавить обязательное поле, которое тут не заполняется

[Владимир Максимов]

Цитата:

Сообщение от trud

Почему ничего - он зарепортит ошибку. Ошибочная запись создана не будет. Могут кстати и не только добавить validateWrite, но и добавить обязательное поле, которое тут не заполняется

Т.е. пользователь выступает как тестер? Разработчик разрабатывает "не глядя" и не тестируя. А тестер - это лишняя профессия... Ну, поздравляю. Вот и Вы, наконец, поняли позицию Microsoft

[wojzeh]

Цитата:

Сообщение от trud

Откуда вы знаете что все ключевые поля заполнены? Для этого и нужен validateWrite, который собственно это проверит.
Я для такого обычно использую следующий метод

public static void validateWriteRecordCheck(Common _record)
    {
        if (! _record.validateWrite())
        {
            throw error(strFmt("Failed to write %1 table", tableId2pname(_record.TableId)));
        }
    }

https://github.com/TrudAX/XppTools/b...xClass/DEV.xml

твой подход тоже противоречит рекомендациям: validateWrite как раз и нужен, чтоб не генерить исключительных ситуаций.

[wojzeh]

Цитата:

Сообщение от Владимир Максимов

validateWrite() нужна в том случае, если

согласно рекомендациям, с коими я согласен, фраза должна строиться от противного:

validateWrite() не нужна лишь в том случае, если...

[Владимир Максимов]

Цитата:

Сообщение от wojzeh

согласно рекомендациям, с коими я согласен, фраза должна строиться от противного:

validateWrite() не нужна лишь в том случае, если...

Хорошо. В данном случае она нужна? Почему?

[trud]

А как вы делаете? Владимир Максимов как я понял предлагает вставлять невалидную запись, ибо пользователь на ошибку все равно не сможет среагировать. По мне так странный подход

[Владимир Максимов]

Да с чего Вы взяли, что запись не валидная? Вот можете внятно объяснить?

У вас запись из кода и дат. Код вставили. Даты вставили. Зачем еще раз проверять факт наличия этого самого кода и дат?

Кастомизация? А Вы что, не будете править код создания если, скажем, новое поле добавите? Переложите бремя тестирования на пользователя?

Вы программно, подчеркиваю еще раз - программно(!), готовите данные для новой записи. Если Вы "вменяемый" разработчик, то вы обязаны подготовить эти данные корректно. С тем, чтобы validateWrite() прошел без ошибок. Но если Вы изначально готовите данные корректно, то зачем Вам себе перепроверять? Или Вы предполагаете, что можете подготовить не корректные данные. А зачем это делать?

[trud]

Корректно это понятие во времени. Сегодня корректно, а завтра некорректно. Т.е. установите вы какое-нибудь решение к примеру, где добавлят обязательное поле. Заполнять его будут в initValue(), которого тоже тут кстати тоже нет.
Сейчас модно использовать RSAT для тестирования. Без принудительного вызова validateWrite() все тесты пройдут, ошибку заменят только впоследствии

[Владимир Максимов]

Т.е. алгоритм такой

1. Сделали кастомизацию
2. Программный код создания записи не исправили по этой кастомизации, т.е. он стал не корректным
3. Тестировать "как положено" не стали
4. В релизе ValidateWrite отловит эту ошибку. Пользователь работать не сможет. Выставит bug-report разработчику

Т.е. сам по себе код создания не корректный. А ValidateWrite нужен для того, чтобы переложить бремя тестирования на пользователя. Ну, тоже стратегия. Все как у Microsoft. С чего, собственно, я и начал...

[wojzeh]

Цитата:

Сообщение от Владимир Максимов

Хорошо. В данном случае она нужна? Почему?

именно о том и был мой исходный пафос, что рассуждать мы (девы) должны так:

в данном случае она НЕ НУЖНА, ПОТОМУ ЧТО

а в большинстве случаев мы ставим валидацию перед записью. comme il faut, о котором ещё пушкин а.с. писал нам из прошлого в будущее

[wojzeh]

Цитата:

Сообщение от trud

А как вы делаете? Владимир Максимов как я понял предлагает вставлять невалидную запись, ибо пользователь на ошибку все равно не сможет среагировать. По мне так странный подход

я просто указал, что не нужно генерить исключительную ситуацию при валидации.

делаем вроде такого

MyTable t;
t.Field1 = 1;
t.Field2 = 'Test';
if (t.validateWrite())
{
t.insert();
}
else
{
error('something went wrong');
}

[wojzeh]

Цитата:

Сообщение от Владимир Максимов

Да с чего Вы взяли, что запись не валидная? Вот можете внятно объяснить?

могу. бест практис как раз о том, что мы не должны париться, валидная она или нет: просто проверяйте.

простой пример:
программно пишем дату, которую получили через параметры. она из прошлого, а мы по логике допускаем только даты из будущего. ну и так далее.

в моём конкретном случае, мне нужно написать extension для проверки дупликатов. было бы написано, как доктор прописал, щас бы уже всё работало, так - делаем выкрутасы.

[trud]

Цитата:

Сообщение от Владимир Максимов

3. Тестировать "как положено" не стали

Я написал, тестируем RSAT, в моем варианте он поймает исключение, в вашем варианте вставится некорректное значение, никто этого не заметит

error('something went wrong');

Это вообще страшный код. тут запись одна, а если транзакция? вставляем хорошие, не вставляем плохие? Вариант Владимира - когда вставляем хоть что-то, в этом случае мне кажется даже лучшим

[wojzeh]

Цитата:

Сообщение от trud

error('something went wrong');

Это вообще страшный код. тут запись одна, а если транзакция? вставляем хорошие, не вставляем плохие? Вариант Владимира - когда вставляем хоть что-то, в этом случае мне кажется даже лучшим

Страшный он или нет, это определяется каждым конкретным сценарием. Я говорю о best practice вообще. Следование им самим же рекомендателем было бы логичным и полезным для разработчиков, вынужденных работать в парадигме extensions.

Твой же пример со статическим вызовом буферного метода и генерацией исключительной ситуации противоречит данной рекомендации, но было бы интересно взглянуть, как ты его используешь на конкретном примере.

[Владимир Максимов]

Цитата:

Сообщение от wojzeh

могу. бест практис как раз о том, что мы не должны париться, валидная она или нет: просто проверяйте.

Может я плохо искал, но в документации по Best Practices я такой рекомендации не нашел

Цитата:

Сообщение от wojzeh

простой пример:
программно пишем дату, которую получили через параметры. она из прошлого, а мы по логике допускаем только даты из будущего. ну и так далее.

Это принципиально другой пример. Параметр - это данные, пришедшие из-вне. Не созданные программистом. Такое, разумеется, надо проверять. Причем не факт, что в validateWrite. Возможно, непосредственно в коде создания записи

Но в примере, который послужил основанием для дискуссии, "внешних" данных нет вообще. Нечего проверять-то...

Цитата:

Сообщение от wojzeh

в моём конкретном случае, мне нужно написать extension для проверки дупликатов. было бы написано, как доктор прописал, щас бы уже всё работало, так - делаем выкрутасы.

Ну, если не рассматривать вопрос о том, что программный контроль уникальности - заведомо не надежен, то выбранный способ решения сам по себе не однозначен. Зависит от постановки задачи. Тут есть разные варианты и использование validateWrite - одно из многих. Не для обсуждения, а просто чтобы показать другие варианты

- Делать проверку на возможный дубликат до вызова метода создания
- Если есть дубль, то менять данные с тем, чтобы дубля не было при создании новой записи

Это так, первое, что в голову пришло. Но могут быть и другие варианты. Причем я не удивлюсь, если позже Вам придется добавлять в ValidateWrite() параметры для того, чтобы исключить ту или иную проверку...

[Владимир Максимов]

Цитата:

Сообщение от trud

Я написал, тестируем RSAT, в моем варианте он поймает исключение, в вашем варианте вставится некорректное значение, никто этого не заметит

Как это не заметит? Вы что, не проверяете, что в результате было создано? Я про это и говорил, что тестирование не корректно - не посмотрели результат теста не в смысле лога ошибок, а сам результат. Что получилось-то?

PS: Что такое RSAT? Автотесты что-ли?