вторник, 1 апреля 2014 г.

Где кончается кодирование и начинается архитектура и проектирование?

"Зачем нам какие-то интерфейсы и диаграммы классов? Это же JavaScript!" К чему часто добавляется аргумент "изобретенный и сделанный за 10 дней" (JavaScript + История создания Javascript)

Согласен, интерфейсы и диаграммы именно в виде картинок (или других графических клубков из линий, квадратиков и кружков) бывают не нужны. Но понимать правила, допущения и ограничения при взаимодействие двух участков кода все таки надо и несколько моих личных десятилетий создания программ вопят, что "кроилово ведет к попадалову", зачастую не разовому, а очень даже регулярному, на несколько лет и с хорошим ростом цены на каждый случай...



Кроилово номер раз:

result.IsEnabledFunctionBody = @"
                    var dropDown = eval(this.control.id + '_DD');
                    if(dropDown) {
                        return dropDown.enabled;
                    }
                    else {
                        return this.control.enabled;
                    }
                ";

Очень понятная и довольно простая задача - определить доступность элемента ввода на редактирование в нем значения.

Хотя и сильно урезанная: из всего множества (я не специалист в этой задаче, но всегда есть варианты) подходов выбран подход на основе значения свойства "enabled". Ну да ладно, вполне себе подход и кто я такой что бы критиковать еще и его? И без этого есть к чему доеб претензии предъявить.

В основном я пишу на C# и первое, что я вижу потерянным - это проверка соответствия интерфейса (или структуры) переданных данных ожиданиям алгоритма. В C# просто так не обратишься к не существующему свойству или к существующему, но не того типа. Обычная ошибка, проверка которой вытащена на этап компиляции кода. Другое дело JS и аргументация "10 дней": типы данных можно пересчитать по пальцам одной руки, а динамическое конструирование интерфейса является одной из основ языка, так же как и привязка к данным во время выполнения. Аргументация, собственно, никакая. Это просто факт, который никак не отменяет устоявшиеся схемы классов, объектов, методов, свойств и интерфейсов взаимодействия (которые и устоялись-то только потому, что в перспективе "пара лет поддержки" с ними все таки выгоднее, чем без них). Просто данное средство разработки не реализует некоторые проверки кода и забота о них переложена на программиста.

В этом коде на программиста оказались переложены вот эти проверки:
- отсутствие свойства "enabled"
- неопределенность типа того значения, которое возвращает свойство "enabled"

В результате, вроде бы простой алгоритм в некоторых случаях станет выдавать результат, не адекватный цели его вызова:
- передали объект у которого нет свойства "enabled"? "да, конечно можно редактировать"
- передали объект у которого свойство "enabled" есть, но возвращает null? "да, конечно можно редактировать"
- передали объект у которого свойство "enabled" есть, но возвращает "undefined"? "да, конечно можно редактировать"
- передали объект у которого свойство "enabled" есть, но возвращает строку (да, идиотский вариант, но технически возможный и разное бывало...)? "да, конечно можно редактировать"

Фактически, вместо адекватного ответа "Я не знаю, думайте сами", этот код абсолютно уверенно делает оторванные от реальности заявления, подводя под монастырь алгоритм, который будет выполняться следующим после него. Есть еще загадочный 'eval' который по некой логике (непостижимой в этом методе) должен для "id +  '_DD'" вернуть какой-то новый объект, у которого следует запросить 'enabled'.

К этому коду я бы приложил еще и тесты на все варианты использования: 
- нет свойства
- свойство возвращает undefined, null или string (который представляет собой все остальное множество неверных значений)
- свойство возвращает true/false

На каждый вариант должно быть спроектировано (удивительно, но даже в такой простой задаче есть проектирование!) поведение и реализовано возвращение соответствующего результата.

Я бы переделал этот код вот так (и следует исключить eval из именно этого метода - это зависимость от черте чего):
result.IsEnabledFunctionBody = @"
                    var dropDown = eval(this.control.id + '_DD');
                    if(this.control) {
                        if(dropDown.enabled !== true && dropDown.enabled !== false) {
                            this.LogOperationError('Only ""true"" or ""false"" values are expected');
                        }
                        else {
                            return dropDown.enabled;
                        }
                    }
                    else {
                        this.LogOperationError('""this.control"" is not initialized');
                    }
                ";
LogOperationError в этом фрагменте кода - это аналог эксепшена в C#. Обрабатывается отдельным кодом после вызова этого алгоритма. Это уже из области задач по архитектуре. А начиналось с банального "control.enabled"...

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

В зависимости от конкретных задач, стоимость поддержки большого количества решений, сделанных в таком ключе, может быть как слишком высокой (попадалово), так и адекватной задачам.

Кроилово номер два (это уже C# и описание косяков в этот раз я добавил прямо в код):

private void Execute(string parameterValue) {
    // !!! parameterValue - нет проверки на string.IsNullOrEmpty. это явно недопустимые значения и их следует считать опечатками при вызове метода.
ITestControl actionTreeList = TestControl; 
IGridBase grid = actionTreeList.GetInterface<IGridBase>();
List<IGridColumn> columns = new List<IGridColumn>(grid.Columns);
if(columns.Count > 0) { // !!! где 'else'??? метод явно не выполнил свою работу, но прикидывается веником
int rowCount = grid.GetRowCount();
for(int i = 0; i < rowCount; i++) {
string cellValue = grid.GetCellValue(i, columns[0]);
if(cellValue == parameterValue) {
actionTreeList.GetInterface<IGridDoubleClick>().DoubleClickToCell(i, columns[0]);
               break;
}
}
       // !!! где проверка что за цикл так и не нашли нужный элемент??? опять же метод явно не выполнил свою работу, но прикидывается веником
}
}

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