Показать сообщение отдельно
Старый 15.03.2008, 18:19
Iv вне форума Посмотреть профиль Отправить личное сообщение для Iv Посетить домашнюю страницу Iv Найти все сообщения от Iv
  № 31  
Iv
 
Аватар для Iv

Регистрация: Apr 2001
Адрес: Moscow
Сообщений: 1,475
Attention Оценка рисков изменения логики

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

Вернемся к коду и оценим риски.
Использование объекта sourceBezier очень кратко и я вижу только одну потенциальную проблему: метод split может изменять объект sourceBezier, что приведет к непредсказуемым последствиям в дальнейшем. Мы должны убедиться в обратном.
Перейдем в метод split и оценим возможность неприятных последствий. В коде метода split отсутствуют действия, изменяющие текущий объект. Но в качестве параметров создания объектов Line передаются точки текущего объекта и существует опасность, что они могут быть изменены методом midpoint.
Переходим на метод midpoint и видим, что с точками не производится изменяющих их операций, поскольку метод Point.interpolate не изменяет аргументы.
Но тут возникает вопрос: а есть ли смысл в методе split создавать объект LineSVG только для того, чтобы в итоге вызвать метод совсем другого класса - Point? Похоже, что нет, ведь объекты LineSVG в методе split ни для чего более не используются.

Решено, перед дальнейшими операциями делаем небольшой рефакторинг по замене метода midpoint на вызов interpolate и получаем:
Код AS3:
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. Так и есть - это моя ошибка. В старом методе нет и намека на то, что должен передаваться существующий объект. Рыдаю, заламываю руки, посыпаю голову пеплом. Нет, не то, боюсь это не поможет. Исправляем ошибку, используя клоны вместо самих точек:
Код AS3:
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.