Портируем, рефакторим, оптимизируем

[Iv]

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

Вернемся к коду и оценим риски.
Использование объекта 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 на предмет аналогичных ошибок. Таких нет, идем дальше.

Вновь возвращаемся к тому, с чего начали: к оценке рисков изменения логики.

[Iv]

Краткое содержание предыдущей серии:
мы хотим вместо sourceBezier использовать source и оцениваем риск того, что метод split может изменять текущий объект.

Итак, благодаря нашим действиям стало очевидно, что split этого не делает. Заменяем. Тестируем.
Конечно, если бы возникли малейшие сомнения, то процесс был бы куда серьезней, да и тестирование потребовало бы больших трудозатрат. В очевидной же ситуации занудствовать не стоит, и мы идем дальше.
Удаляем создание объекта sourceBezier и заменяем вхождение на source:

var halves:Array = source.split();

Рекурсивные вызовы обеспечивают разделение странных кривых, задаваемых тремя точками одной кривой и одной точкой другой. Что-то здесь не так. Посмотрев внимательно убеждаемся в том, что source.start это firstHalf.start, а source.end это secondHalf.end - а иначе и быть не может, потому, что если разделить одну кривую на две, то стартовая точка будет одной и той-же у исходной и у первой результирующей кривой, а конечная точка будет одной и той же у исходной и второй результирующей кривой.
После замены рекурсивные вызовы будут выглядеть так:

getQuadBez_RP (firstHalf.start, firstHalf.startControl, firstHalf.endControl, firstHalf.end, k, qcurves);
getQuadBez_RP(secondHalf.start, secondHalf.startControl, secondHalf.endControl, secondHalf.end, k, qcurves);

Тестируем, убеждаемся в том, что ничего не перепутали.

Можем вернуться к рефакторингу.

[Iv]

Как мы помним, наша попытка заменить создание нового объекта на использование this провалилась потому, что рекурсивные методы вызывались не из нужного объекта. После того, как мы заменили передаваемые аргументы на точки одного объекта, нам стало понятно от чьего имени должна происходить рекурсия.

Устанавливаем правильные объекты для рекурсивного метода и заменяем создание нового объекта на this:

const source:CubicBezierSVG = this;
.....
firstHalf.getQuadBez_RP (....
secondHalf.getQuadBez_RP(....

Тестируем. Сработало!

Редактор нам подсветил неиспользуемые переменные. Удаляем их, сохраняем документ. Панель Problems насыпала проблем. Проходим по всем ошибочным вызовам, удаляя в каждом первые четыре аргумента. Делаем это до тех пор, пока панель Problems не прекратит истерику.

Результатом наших трудов стало то, что метод getQuadBez_RP приведен в такое состояние:

public function getQuadBez_RP(k:Number, qcurves:Array):void {
	const source:CubicBezierSVG = this;
	// find intersection between bezier arms
	var startArm : LineSVG = new LineSVG(source.start, source.startControl);
	var endArm : LineSVG = new LineSVG(source.endControl, source.end);
	var s:Point = startArm.getLineIntersection(endArm);
	// find distance between the midpoints
	var dx:Number = (source.start.x + source.end.x + s.x * 4 - (source.startControl.x + source.endControl.x) * 3) * .125;
	var dy:Number = (source.start.y + source.end.y + s.y * 4 - (source.startControl.y + source.endControl.y) * 3) * .125;
	// split curve if the quadratic isn't close enough
	if (dx*dx + dy*dy > k) {
		var halves:Array = source.split();
		var firstHalf:CubicBezierSVG = halves[0] as CubicBezierSVG; 
		var secondHalf:CubicBezierSVG = halves[1] as CubicBezierSVG;
		// recursive call to subdivide curve
		firstHalf.getQuadBez_RP (k, qcurves);
		secondHalf.getQuadBez_RP(k, qcurves);
	} else {
		// end recursion by saving points
		qcurves.push({p1x:source.start.x, p1y:source.start.y, cx:s.x, cy:s.y, p2x:source.end.x, p2y:source.end.y});
	}
}

и это состояние нас не устраивает тем, что в конце кода присутствует вопиющий пример неявного нетипизированного доступа. Им и займемся.

[Iv]

Загадочная буква q в имени массива, а также имена в создаваемом объекте нас наталкивают на мысль о том, что этот объект по-сути - кривая Безье второго порядка.
Чтобы в следующий раз не вспоминать что-же там в массиве, переименуем аргумент qcurves в quadraticCurves и исправим появившиеся ошибки. Можно даже не тестировать после этого. Хотя, если вы не параноик, это вовсе не значит, что за вами никто не следит. Можете протестировать на всякий случай.

Создадим класс, определим методы доступа к управляющим точкам, а так-же реализуем методы доступа к данным, одноименных с используемыми в объекте:

package com.itechnica.svg {
	import flash.geom.Point;		
 
	public class QuadraticBezierSVG {
 
		private var startPoint:Point;
		private var controlPoint:Point;
		private var endPoint:Point;
 
		public function QuadraticBezierSVG(start:Point, control:Point, end:Point) {
			initInstance(start, control, end);
		}
 
		private function initInstance(start:Point, control:Point, end:Point):void {
			startPoint = start;
			controlPoint = control;
			endPoint = end;
		}
 
		public function get start ():Point {
			return startPoint;
		}
		public function set start (value:Point):void {
			startPoint = value;
		}
 
		public function get control ():Point {
			return controlPoint;
		}
		public function set control (value:Point):void {
			controlPoint = value;
		}
 
		public function get end ():Point {
			return endPoint;
		}
		public function set end (value:Point):void {
			endPoint = value;
		}
 
// это методы доступа к данным, одноименные с примененными в объекте:
		public function get p1x ():Number {
			return start.x;
		}
		public function get p1y ():Number {
			return start.y;
		}
 
		public function get cx ():Number {
			return control.x;
		}
		public function get cy ():Number {
			return control.y;
		}
		public function get p2x ():Number {
			return end.x;
		}
		public function get p2y ():Number {
			return end.y;
		}
 
 
	}
}

Вернемся в метод getQuadBez_RP, продублируем строку с push, одну закомментируем.
Затем перед этими строками инициализируем управляющие точки и кривую безье второго порядка:

var quadraticStart:Point = new Point(source.start.x, source.start.y);
var quadraticControl:Point = new Point(s.x, s.y);
var quadraticEnd:Point = new Point(source.end.x, source.end.y);
var quadraticBezier:QuadraticBezierSVG = new QuadraticBezierSVG(quadraticStart, quadraticControl, quadraticEnd);
quadraticCurves.push(quadraticBezier);
// quadraticCurves.push({p1x:source.start.x, p1y:source.start.y, cx:s.x, cy:s.y, p2x:source.end.x, p2y:source.end.y});

Тестируем. Всё прекрасно работает.

[Iv]

Если объект создавался, значит, что его кто-то использовал. И это был нетипизированый доступ. Включаем отдел головного мозга, ответственный за Шерлока Холмса и проводим частное расследование. Задача - выяснить кто использует массив, передаваемый в аргументах.

Выделяем имя метода getQuadBez_RP и применяем волшебную комбинацию CTRL+R. В открывшейся панели поиска видим, что метод вызывается в этом же классе (рекурсия) и в классе PathToArray в методе makeDrawCmds.
Используя панель Search переходим на makeDrawCmds, сразу попадаем на первое вхождение использования метода. Немедленно сильно пугаемся кода: огромный метод, непонятные имена переменных, использование в case загадочных строковых данных, if-ы и while-ы непонятной глубины вложенности. И нам всё это придется разгрести.

Видим, что всего вхождений четыре. Видим также, что переменная, хранящая ссылку на наш массив, здесь называется qc. Перейдем на объявление массива, видим, что это локальная переменная. Для начала поиском и заменой переименуем ее в quadraticCurves, чтобы появилась визуальная связь с массивом в методе getQuadBez_RP - ведь это один и тот-же массив.
Заодно пора переименовать и сам метод getQuadBez_RP. Назовем его без сокращений и постараемся в имени отразить его деятельность. Переименовываем в toQuadraticBezierArray.
Полученная строка source.toQuadraticBezierArray тоже не очень понятна, поэтому переименуем переменную source в сubicBezier.
Теперь, если кто-то увидит строку кода "сubicBezier.toQuadraticBezierArray(...", то ему будет намного понятнее что происходит.

[Iv]

Краткое отступление: вы не думайте, что код, написанный Helen плох. Для своего времени это прекрасный код.
Просто в то время цели, которых добивались от кода, были совсем другими. Никто даже не думал о том, чтобы код был простым и читабельным. Более того, лучшим считался тот программист, который был способен написать много непонятного кода и, при этом, умудриться самому в нем не запутаться.
Еще один очень вероятный фактор: Helen привела к такому виду код в процессе оптимизации, а мы сейчас идем в обратном направлении.

К тому-же, попробуйте написать проект во Flash IDE и поймете, что первопроходцы Flash тех времен - герои.

Но так или иначе, в результате с тех доисторических времен до нас дошло много кода, но никто не знает что и как он делает.

Сейчас ситуация в корне изменилась: не назовут программиста хорошим, если не понятно что и как делает его код. Имейте это ввиду.

[Iv]

В классе PathToArray мы видим четыре места, в которых используется вызов метода toQuadraticBezierArray.
Внешне код очень похож и, возможно, речь идет об одинаковом коде. Проверим.

Первым делом жмем волшебную комбинацию: CTRL+SHIFT+F. Код отформатируется, расставятся пробелы и всё такое.
Теперь, если строки одинаковы, то они будут находиться поиском.
Начнем со строки

quadraticCurves = [];

Пусть это будет первая строка, далее строки буду обозначать номерами.
Чтобы после поиска было удобно возвращаться в начало, правым кликом по серому полю окна слева добавьте закладку и затем используйте ее для возврата: слева, рядом со скроллером можно будет кликнуть на нее.
Итак,
1 - совпадает
2 - не совпадает
3 - совпадает
4 - не совпадает, хотя вроде должна. Ага, вынесем объявление переменной ii в начало метода. CTRL+SHIFT+F, проверяем, теперь совпадает.
После цикла две строки не совпадают и только последняя еще раз совпадает.

Это очень хороший результат: мы сможем вынести совпадающие части в отдельный метод, что, возможно, избавит от нас от необходимости вносить изменения в четырех местах.
Поэтому временно откладываем рефакторинг скрытого нетипизированного доступа и приступаем и извлечению метода.

Опускаем строку присвоения массива под строку создания сubicBezier, затем копируем участок кода. Создаем новый метод и вставляем код туда:

private function extractedCode():void {
	quadraticCurves = [];
	сubicBezier.toQuadraticBezierArray(1, quadraticCurves);
	for (ii = 0;ii < quadraticCurves.length; ii++) {
		drawCmds.push(['C', [quadraticCurves[ii].cx, quadraticCurves[ii].cy, quadraticCurves[ii].p2x, quadraticCurves[ii].p2y]]);
	}
}

Смотрим, в каких данных мы нуждаемся:
- массив quadraticCurves можем объявить прямо здесь, объявляем;
- сubicBezier должны получить, добавляем аргумент;
- итератор цикла объявляем здесь же, и переименовываем из двойного ii в одинарную i.

Имеем вот такую картину:

private function extractedCode(сubicBezier:CubicBezierSVG):void {
	const quadraticCurves:Array = [];
	сubicBezier.toQuadraticBezierArray(1, quadraticCurves);
	for (var i:int = 0;i < quadraticCurves.length; i++) {
		drawCmds.push(['C', [quadraticCurves[i].cx, quadraticCurves[i].cy, quadraticCurves[i].p2x, quadraticCurves[i].p2y]]);
	}
}

Возвращаемся по метке, добавляем после создания сubicBezier вызов метода:

extractedCode(сubicBezier);

Тестируем. Всё в порядке. Аналогично заменяем следующие участки кода на вызов метода, не забывая каждый раз тестировать.
По окончании, редактор нам подскажет, что появились две неиспользованные переменные. Удалим их.
Имя метода никуда не годится, назовем его pushCubicBezierDrawCommands.

[Iv]

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

private function pushCubicBezierDrawCommands(сubicBezier:CubicBezierSVG):void {
	const quadraticCurves:Array = [];
	сubicBezier.toQuadraticBezierArray(1, quadraticCurves);
	for (var i:int = 0;i < quadraticCurves.length; i++) {
		var curve:QuadraticBezierSVG = quadraticCurves[i] as QuadraticBezierSVG;
		if (curve!=null) {
			drawCmds.push(['C', [curve.cx, curve.cy, curve.p2x, curve.p2y]]);
		} else {
			throw new Error("com.itechnica.svg.PathToArray.pushCubicBezierDrawCommands: incorrect curve type"); 
		}
	}
}

Тестируем.

Заменяем доступ к свойствам с непонятными именами на доступ через управляющие точки:

drawCmds.push(['C', [curve.control.x, curve.control.y, curve.end.x, curve.end.y]]);

Возможно, свойства добавленные в класс QuadraticBezierSVG из ранее использовавшегося объекта, больше нигде не используются. Проверим.
Закомментируем все эти свойства. Редактор не видит ошибок. Протестируем. С тем же успехом. Отлично, эти свойства можем удалять.

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

[Iv]

Скопируем "drawCmds.push", перейдем в начало документа и пройдемся поиском, чтобы посмотреть, что в принципе добавляется в массив. Видим, что всегда это буква и массив данных. Эта информация в сочетании с именем массива, которое не говорит, а намекает смекалистым на то, что в нем хранятся команды рисования, позволяет нам придумать имя для будущего класса объекта данных. Мы назовем его DrawingCommand.

Создаем класс DrawingCommand.

package com.itechnica.svg {
 
	public class DrawingCommand {
 
		private var commandType:String;
 
		public function DrawingCommand(type:String, ...args) {
			initInstance(type, args);
		}
 
		private function initInstance(type:String, args:Array):void {
			commandType = type;
 
		}
	}
}

И на этом месте мы вынуждены остановиться. Как мы видели, в зависимости от различных значений буквы, сущность и количество переменных может быть разным. И по складывающейся логике мы должны в методе initInstance прописать switch, в котором будем делать выбор действий.

В таком раскладе мне это категорически не нравится. Вместо массива мы получим объект данных мало чем по своей практичности отличающийся от него. При необходимости им воспользоваться нам придется каждый раз вначале выяснять как мы можем им воспользоваться проверяя его тип. От этого впоследствии нам придется избавляться с помощью замены условных операторов полиморфизмом. Так не проще ли сейчас грамотно реализовать логику, чтобы потом не возвращаться? Ведь эту часть мы делаем с нуля.
Так и поступим.

Кратко опишу чего мы будем добиваться: мы сделаем для каждой команды рисования свой класс. Чтобы эти классы правильно типизировались и могли иметь общую логику, отнаследуемся от класса DrawingCommand. Чтобы создавать экземпляры этих классов, сделаем DrawingCommand фабрикой.

Еще раз вернемся к коду и посмотрим, какие именно классы нам придется создавать. В этом нам очень поможет метод getShapes класса SVGDisplayInFlash, поскольку в нем компактно использованы все варианты применения.
Удаляем всё содержимое класса, копируем блок switch из getShapes и закомментируем его.
Поскольку у нас будет целая группа классов, то создаем в папке svg папку draw и перемещаем туда класс.
Выглядит сейчас он так:

package com.itechnica.svg.draw {
 
	public class DrawingCommand {
 
//		switch (d[0]) {
//			case "F" :
//				drawTarget.beginFill(dp[0], dp[1]);
//				break;
//			case "S" :
//				drawTarget.lineStyle(dp[0], dp[1], dp[2]);
//				break;
//			case "M" :
//				drawTarget.moveTo(dp[0], dp[1]);
//				break;
//			case "L" :
//				drawTarget.lineTo(dp[0], dp[1]);
//				break;
//			case "C" :
//				drawTarget.curveTo(dp[0], dp[1], dp[2], dp[3]);
//				break;
//		}
 
	}
}

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

public static const FILL:String = "F";
public static const STYLE:String = "S";
public static const MOVE:String = "M"; 
public static const LINE:String = "L";
public static const CURVE:String = "C";

И идем заменять в коде соответствующие строки на константы.
Чтобы это сделать быстро и качественно, используем File Search. Но перед этим, чтобы ограничить поиск только нужными папками создадим новый workspace с именем SVGToFlashSrc, в который включим только файлы, лежащие в папке src.
- выделяем строку "F" (с кавычками);
- CTRL+H, выделенная строка должна оказаться в поле ввода
- выбираем workspace с именем SVGToFlashSrc;
- жмем кнопку Replace...
- в поле with вводим: DrawingCommand.FILL
- далее шаг за шагом заменяем, однако не делаем этого в вызовах String2.replace(...) потому, что если заменим, то бардак там будет полным. Ибо чует сердце, что единообразие вызовов этих методов нам еще понадобится.

[Iv]

Общий вид фабричного метода получается таким:

package com.itechnica.svg.draw {
 
	public class DrawingCommand {
 
		public static const FILL:String = "F";
		public static const STYLE:String = "S";
		public static const MOVE:String = "M"; 
		public static const LINE:String = "L";
		public static const CURVE:String = "C";
 
		public static function create(type:String, ...args):DrawingCommand {
			switch (type) {
				case FILL:
					return new FillCommand(args[0], args[1]);
				break;
 
				case STYLE:
					return new StyleCommand(args[0], args[1], args[2]);
				break;
 
				case MOVE:
					return new MoveCommand(args[0], args[1]);
				break;
				case LINE:
					return new LineCommand(args[0], args[1]);
				break;
				case CURVE:
					return new CurveCommand(args[0], args[1], args[2], args[2]);
				break;
				default:
					throw new Error("com.itechnica.svg.draw.DrawingCommand.create("+type+", ...args) - unknown type");
			} 
		}
 
//		switch (d[0]) {
//			case FILL :
//				drawTarget.beginFill(dp[0], dp[1]);
//				break;
//			case "S" :
//				drawTarget.lineStyle(dp[0], dp[1], dp[2]);
//				break;
//			case "M" :
//				drawTarget.moveTo(dp[0], dp[1]);
//				break;
//			case "L" :
//				drawTarget.lineTo(dp[0], dp[1]);
//				break;
//			case "C" :
//				drawTarget.curveTo(dp[0], dp[1], dp[2], dp[3]);
//				break;
//		}
	}
}

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

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