![]() |
|
||||||||||
|
|||||
|
Регистрация: Apr 2001
Адрес: Moscow
Сообщений: 1,475
|
Рефакторинг как процесс - ряд мелких эквивалентных шагов не изменяющих логику приложения. Если мы хорошо понимаем эквивалентность каждого конкретного шага, то достаточно общего тестирования проекта.
Ситуация резко изменяется в тот момент, когда мы вносим изменения в логику. Тем более, если изменения вносятся в чужой код. В этом случае оценка рисков и тестирование должны быть куда более глубокими. Вернемся к коду и оценим риски. Использование объекта sourceBezier очень кратко и я вижу только одну потенциальную проблему: метод split может изменять объект sourceBezier, что приведет к непредсказуемым последствиям в дальнейшем. Мы должны убедиться в обратном. Перейдем в метод split и оценим возможность неприятных последствий. В коде метода split отсутствуют действия, изменяющие текущий объект. Но в качестве параметров создания объектов Line передаются точки текущего объекта и существует опасность, что они могут быть изменены методом midpoint. Переходим на метод midpoint и видим, что с точками не производится изменяющих их операций, поскольку метод Point.interpolate не изменяет аргументы. Но тут возникает вопрос: а есть ли смысл в методе split создавать объект LineSVG только для того, чтобы в итоге вызвать метод совсем другого класса - Point? Похоже, что нет, ведь объекты LineSVG в методе split ни для чего более не используются. Решено, перед дальнейшими операциями делаем небольшой рефакторинг по замене метода midpoint на вызов interpolate и получаем: public function split():Array { const startMidpoint:Point = Point.interpolate(startPoint, startControlPoint, 0.5); const middleMidpoint:Point = Point.interpolate(startControlPoint, endControlPoint, 0.5); const endMidpoint:Point = Point.interpolate(endControlPoint, endPoint, 0.5); const startMiddleMidpoint:Point = Point.interpolate(startMidpoint, middleMidpoint, 0.5); const middleEndMidpoint:Point = Point.interpolate(middleMidpoint, endMidpoint, 0.5); const centerMidpoint:Point = Point.interpolate(startMiddleMidpoint, middleEndMidpoint, 0.5); return [ new CubicBezierSVG(startPoint, startMidpoint, startMiddleMidpoint, centerMidpoint), new CubicBezierSVG(centerMidpoint, middleEndMidpoint, endMidpoint, endPoint) ]; } Раз уж мы больше в этом методе не применяем midpoint, то проверим, может быть больше он не нужен никому? Проверяем, убеждаемся в ненужности, удаляем, тестируем. Еще раз вернемся к методу split. Дело в том, что здесь я заметил одну нехорошую особенность: при создании возвращаемых кривых, в качестве параметров передаются startPoint и endPoint текущего объекта. Это значит, что если изменится стартовая точка первой возвращаемой кривой или конечная точка второй, то изменится текущая кривая. Поскольку мы не знаем, что дальше будет происходить с возвращаемыми кривыми, то желательно застраховаться от таких неприятностей, передавая не точки кривой, а ее клоны. Но можем ли мы сделать это прямо сейчас? Достаточно ли у нас информации и возможностей? Ведь не исключено, что это запланированное поведение метода и так и должно быть. Что-то мне подсказывает, что, возможно, это мы внесли такое поведение в код. Для того, чтобы проверить, открываю исходный проект, класс Math2, метод bezierSplit. Так и есть - это моя ошибка. В старом методе нет и намека на то, что должен передаваться существующий объект. Рыдаю, заламываю руки, посыпаю голову пеплом. Нет, не то, боюсь это не поможет. Исправляем ошибку, используя клоны вместо самих точек: return [ new CubicBezierSVG(startPoint.clone(), startMidpoint, startMiddleMidpoint, centerMidpoint), new CubicBezierSVG(centerMidpoint, middleEndMidpoint, endMidpoint, endPoint.clone()) ]; Раз уж такое произошло однажды, значит, где-то в коде имеется аналогичная проблема. Проверяем методы классов CubicBezierSVG и LineSVG на предмет аналогичных ошибок. Таких нет, идем дальше. Вновь возвращаемся к тому, с чего начали: к оценке рисков изменения логики.
__________________
http://realaxy.com Последний раз редактировалось iNils; 20.12.2010 в 13:28. |
![]() |
Часовой пояс GMT +4, время: 10:16. |
|
|
« Предыдущая тема | Следующая тема » |
|
|