PDA

Просмотр полной версии : Портируем, рефакторим, оптимизируем


Iv
10.03.2008, 15:46
В нескольких топиках обсуждались вопросы оптимизации кода и возникла идея рассмотреть это на конкретном примере: взять готовый код, причесать и оптимизировать. Поскольку предложенный пакет классов на AS2, то добавился еще один шаг - портирование на AS3.

Собственно код, который будем использовать взят с сайта Helen Triolo: http://www.flash-creations.com/notes/sample_svgtoflash.php
Немаловажным моментом является то, что Helen разрешает (даже приветствует) подобные действия с ее кодом и своими действиями мы не нарушаем ее законных интересов.


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

- до полного окончания прошу никого в этот топик не писать; Если вы хотите получать уведомления, просто в левом верхнем углу кликните на иконку "Подписаться на эту тему".
- я выкладываю код - результат очередного этапа, с пояснениями что сделано и зачем.
- предложения, вопросы, обнаруженные ошибки участники скидывают мне на мыло: ivan.dembicki на gmail, с сабжем "Flasher.ru - Портируем, рефакторим, оптимизируем".
- мы их приватно обсуждаем, затем я резюмирую здесь результат, разумеется, с указанием авторства.
- выкладывается исправленный код с пояснениями что сделано и зачем.
- после этого переходим к следующему этапу.

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

Процесс делим на этапы:
I. Портируем
II. Рефакторим
III. Оптимизируем

Поехали.

Iv
10.03.2008, 16:15
Заходим на сайт http://www.flash-creations.com/notes/sample_svgtoflash.php идем в конец страницы, открываем окно downloads и скачиваем себе на комп svgtoflash.zip - он 11-й по списку.

Создаем новый AS3 проект (я использую FDT и вам рекомендую), называем его SVGToFlash, затем в винде открываем папку SVGToFlash и закидываем в нее содержимое ZIP файла. В FDT выделяем папку SVGToFlash и жмем F5.
Должна получиться структура файлов как на прилагаемой картинке.
19008

Iv
10.03.2008, 16:57
Теперь мы можем приступисть к этапу портирования проекта на AS3.

Задача этого этапа:
при минимальном количестве изменений в проекте сделать его работоспособным.

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

Iv
10.03.2008, 18:01
Подготовка FLA файла.

Открываем во Flash файл svg_displayinflash.fla.
Изменяем настройки файла:
File - Publish Settings - Flash
устанавливаем FlashPlayer 9 и ActionScript 3.0
Заходим в настройки ActionScript 3.0 и устанавливаем все галочки кроме Strict Mode.
OK.OK.

Затем добавляем папку src в путь к классам:
Edit - Prefences - ActionScript - Actionscript 3.0
добавляем строку ./src и выставляем ее второй по порядку. OK.

Выделяем кадр actions и закомментируем единственную строку кода
// #include "svg_displayinflash.as"

Затем задаем Document class: SVGDisplayInFlash, Flash ругнется отвечаем OK.

На этом пока заканчиваются изменения во FLA файле, сохраняем.

Переходим в FDT,
в проекте создаем папку src, добавляем ее в class path: правый клик на папке, Source Folder - Add to Classpath.

Создаем в этой папке новый класс: SVGDisplayInFlash - если помните, мы класс с этим именем задали как Document Class.
Поскольку у нас в руте есть именованые объекты holder и msg, инициализируем их в этом классе:
package {
import flash.display.MovieClip;
import flash.text.TextField;

public class SVGDisplayInFlash extends MovieClip {

private static const HOLDER_NAME:String = "holder";
private static const MESSAGE_NAME:String = "msg";

private var holderMc:MovieClip;
private var messageTxt:TextField;

public function SVGDisplayInFlash() {
initInstance();
}

private function initInstance():void {
trace("SVGDisplayInFlash.initInstance()");
initStageObjects();
}

private function initStageObjects():void {
holderMc = this[HOLDER_NAME] as MovieClip;
messageTxt = this[MESSAGE_NAME] as TextField;
trace("SVGDisplayInFlash.initStageObjects()");
trace("\t", holderMc, messageTxt);
}
}
}

Сейчас, если мы во Flash скомпилируем проект, то в окне output получим вот такой текст:

SVGDisplayInFlash.initInstance()
SVGDisplayInFlash.initStageObjects()
[object MovieClip] [object TextField]

Добейтесь того, чтобы всё стработало именно так.

Iv
10.03.2008, 18:17
Для того, чтобы файлы AS2 нам не мешались, создадим папку temp и забросим все AS2 файлы в эту папку.

Затем создадим иерархию папок com.itechnika.svg и создадим пустые классы Math2, PathToArray, String2.

В итоге должна получиться вот такая картина происходящего:

(спасибо, iNils за помощь в публикации материалов)

Iv
10.03.2008, 20:20
Порядок портирования классов определяестя следующим образом: вначале портируем классы, которые не требуют других классов проекта.

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

Следуя этой логике, портирование классов начнем с Math2.
Для этого откроем созданный нами пустой класс Math2, и рядом AS2 версию этого класса.
Скопируем содержимое AS2 класса кроме первой и последней строки и вставим в тело AS3 класса.

Оценим степень разрухи: всё не так уж и страшно. Явных ошибок FDT не нашел, только подсветил отсутствие или неверную типизацию.

На этом этапе мы не будем приводить этот код в порядок, поскольку наши изменения могут повлиять на другие классы.
Проделаем аналогичную операцию с остальными классами в папке svg.
Класс String2, а его мы портируем вторым, также не доставил никаких хлопот.
В классе PathToArray потребовалось только импортировать XMLNode:
import flash.xml.XMLNode;

Тестирование.
На данном этапе мы не можем качественно протестировать соответствие кода стандартам AS3. Но минимальный тест всё-таки сделать необходимо.

Поскольку класс PathToArray использует и Math2 и String2, то нам для теста достаточно создать экземляр такого класса для того, чтобы компилятор Flash сказал свое веское слово.

Для того, чтобы создать экземпляр класса PathToArray нам потребуется посмотреть с какими параметрами он вызывается.
С этой целью найдем исходный ZIP файл, распакуем его в папочку на рабочем столе, откроем класс PathToArray и добавим трейсы:
public function PathToArray(svgNode:XMLNode, dCmds:Array) {
trace(svgNode)
trace("-----------")
trace(dCmds)
trace("=====================")
..................
Компилим AS2 проект, видим, что первый параметр - узел XMLNode (копируем один из них, что покороче), а второй параметр может быть пуст (это запомним).

Теперь можем добавить тест. В классе SVGDisplayInFlash добавим метод testPathToArray:private function testPathToArray():void {
const path:String = "<path fill='#ED1C24' d='M261.5,52.3c11.9,3.2-5.2,21,32.9,31.6C288.5,35.5,261.5,52.3,261.5,52.3z' />";
const foo:XMLDocument = new XMLDocument(path);
new PathToArray(foo.firstChild, []);
} и добавим его вызов в методе initInstance.

Результатом компиляции станет ругань компилятора. При ближайшем рассмотрении видим, что нет ничего страшного: дважды он ругается на то, что Number не может быть undefined и несколько раз на дублирование объявления переменных. Это несложно исправить.
В соответствующих местах заменяем проверку на равенство undefined на функцию isNaN:
// if (thisColor == undefined) {
if (isNaN(thisColor)) {
И удаляем лишние var в дублирующихся объявлениях переменных в классах SVGDisplayInFlash и Math2.
Компилируем еще раз и убеждатемся в том, что компилятор не выдает ошибок.
Если это так, то можем переходить к следующей части.

Iv
11.03.2008, 02:24
На этом этапе мы портируем svg_displayinflash.as, который представляет собой код инициализации приложения.
Код состоит из двух функциональных частей: загрузка данных в SVG формате и отрисовка полученного пути. Загрузка данных - дело известное, начнем с этого. Добавим в класс SVGDisplayInFlash методы инициализации загрузки и обработчик полученных данных: private function getSVGData(url:String):void {
const loader:URLLoader = new URLLoader();
loader.addEventListener(Event.COMPLETE, onSVGLoaded);
loader.load(new URLRequest(url));
}

private function onSVGLoaded(event:Event):void {
const loader:URLLoader = event.target as URLLoader;
const data:String = loader.data as String;
const svg:XML = new XML(data);
trace(svg.toXMLString());
}

Обратите внимание на то, что мы можем немедленно протестировать загрузку данных, что и сделаем, добавив в initInstance вызов метода getSVGData, передав ему в качестве параметра url файла данных:
getSVGData(SVG_DATA_URL);

Если в результате компиляции не возникло проблем, то можем переходить к следующему шагу: портированию метода getShapes и процедуре его вызова.
Копируем и вставляем этот метод в класс SVGDisplayInFlash и видим, что FDT подсвечивает множество ошибок. Как это ни странно, но это очень хороший признак, что основная масса ошибок пришлась на последний этап портирования и говорит о правильно избранной стратегии действий. Сейчас у нас развязаны руки и мы можем вносить правки в код, не волнуясь о том, что это может чем-то навредить остальной части проекта. Однако мы не забываем, что на этом этапе наша задача - сделать код работоспособным с минимальными изменениями.
Копируем и вставляем объявления переменных, делаем их приватными. Если мы внимательны, то увидим, что переменная x будет конфликтовать со свойством мувиклипа, в котором мы ее объявили. Если мы невнимательны, то компилятор Flash об этом нам напомнит при первом же тесте.
Переименуем ее в svg: private var svg:XMLDocument = new XMLDocument();
private var conv:PathToArray;
private var paths:Array;
private var nPathNodes:Number; Затем заменяем создание мувиклипа на добавление спрайта и получаем ссылку на объект graphics: // holder.createEmptyMovieClip("p"+iPath, iPath+1);

const drawLayer:Sprite = new Sprite();
holderMc.addChild(drawLayer);
const drawTarget:Graphics = drawLayer.graphics; После этого заменяем все обращения holder["p"+iPath] на drawTarget.

Приступаем к портированию обработки загруженных данных. Пока мы можем пропустить обработку ошибок, поэтому просто переносим строки инициализации из оригинального метода xmlLoaded в метод onSVGLoaded: msg.text = "";
paths = x.firstChild.firstChild.childNodes;
nPathNodes = paths.length;
getShapes(0); Заменяем msg на messageTxt и x на svg.
Добавляем парсинг полученных данных перед первым обращением к объекту svg: svg.ignoreWhite = true;
svg.parseXML(data); Вот то, что в итоге должно было быть добавлено в класс: private var svg:XMLDocument = new XMLDocument();
svg.ignoreWhite = true;
private var conv:PathToArray;
private var paths:Array;
private var nPathNodes:Number;

private function onSVGLoaded(event:Event):void {
const loader:URLLoader = event.target as URLLoader;
const data:String = loader.data as String;

messageTxt.text = "";

svg.parseXML(data);
paths = svg.firstChild.firstChild.childNodes;
nPathNodes = paths.length;

getShapes(0);
}

private function getShapes(iPath:Number):void {
var dCmds:Array = new Array();
var d:Array;
var dp:Array;

conv = new PathToArray(paths[iPath], dCmds);

// draw the shapes in clips in holder movieclip
// holder.createEmptyMovieClip("p"+iPath, iPath+1);

const drawLayer:Sprite = new Sprite();
holderMc.addChild(drawLayer);
const drawTarget:Graphics = drawLayer.graphics;

for (var i:Number=0; i<dCmds.length; i++) {
d = dCmds[i];
dp = d[1];
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;
}
}
// repeat til all paths have been read
if (++iPath < nPathNodes) getShapes(iPath);
} Тестируем изменения. Если видим на экране попугая, то всё хорошо. Если нет, а вполне возможно что в ходе объяснений я что-то пропустил или был неточен, то для такого случая я приаттачу необходимые файлы.

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

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

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

Iv
11.03.2008, 18:39
Итак, после успешного портирования мы переходим к следующему этапу - рефакторингу.

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

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

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

Обратите внимание на то, что здесь ни слова не говорится об увеличении производительности кода, поскольку этот процесс, как правило, затрагивает логику программы, что на этапе рефакторинга недопустимо.

Знающие люди могут возразить против самой постановки задачи: рефакторинг не один большой этап, а регулярно выполняемая процедура в процессе программирования.
Я, конечно в курсе, но первый рефакторинг после портирования - довольно цельный и объемный этап работ не сопоставимый с обычным рефакторингом, проводящимся в процессе написания программы. Именно поэтому мы уделяем ему такое внимание, что даже выделяем в отдельный этап.

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

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

А самой первой задачей рефакторинга станет наведение элементарного порядка в коде, до такой степени, что FDT перестанет нам подсвечивать ошибки.

Iv
11.03.2008, 19:53
Первый класс, который мы поправим - String2. Он самый короткий, и не использует других наших классов. И для примера я поподробнее остановлюсь на самом процессе.

Подсвечиваются всего три ошибки: в методе shrinkSequencesOf не задана типизация локальных переменных. Зададим ее:
var len:int = s.length;
var idx:int = 0;
var idx2:int = 0;
var rs:String = "";

Затем все объявления var заменим на const (пользуйтесь возможностью автоматического поиска и замены). Затем все переменные, которые подсветили ошибку присвоения, обратно заменим на var.

Займемся именованием. Для начала заменим имена аргументов функции. Думать тут особо не надо, поскольку автор позаботилась о комментариях: s заменяем на originalString, ch заменяем на characterToBeFound.

Затем, поскольку сокращений мы не любим, заменим:
len на length
idx на startIndex
idx2 на endIndex
rs на resultString

Также добавим фигурные скобки вложенному while.

Давайте сравним, что было и что стало:

// БЫЛО
public static function shrinkSequencesOf(s:String, ch:String):String {
var len = s.length;
var idx = 0;
var idx2 = 0;
var rs = "";

while ((idx2 = s.indexOf(ch, idx) + 1) != 0) {
// include string up to first character in sequence
rs += s.substring(idx, idx2);
idx = idx2;

// remove all subsequent characters in sequence
while ((s.charAt(idx) == ch) && (idx < len)) idx++;
}
return rs + s.substring(idx, len);
}

// СТАЛО
public static function shrinkSequencesOf(originalString:String, characterToBeFound:String):String {
const length:uint = originalString.length;
var startIndex:int = 0;
var endIndex:int = 0;
var resultString:String = "";

while ((endIndex = originalString.indexOf(characterToBeFound, startIndex) + 1) != 0) {
// include string up to first character in sequence
resultString += originalString.substring(startIndex, endIndex);
startIndex = endIndex;

// remove all subsequent characters in sequence
while ((originalString.charAt(startIndex) == characterToBeFound) && (startIndex < length)) {
startIndex++;
}
}
return resultString + originalString.substring(startIndex, length);
}

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

Iv
11.03.2008, 21:53
Следующий класс для причесывания - PathToArray. Он хоть и дает большое количество ошибок, однако подавляющее большинство - по одинаковой причине, и их можно исправить без существенных усилий.

В методе extractCmds динамически создается объект, содержащий перечень значений стандартных цветов. Эти цвета неизменны, а объект может быть использован неоднократно. Это прекрасный повод сделать объект статической константой и инициализировать его в теле класса. Так и поступим.
Вырезаем из метода всю инициализацию объекта colors, до строчки yellowgreen : 0x9acd32 включительно и вставляем с тело класса непосредственно после объявления класса.
Затем изменяем объявления и переносим закрывающую скобку в конец присвоений значений цветам. Должно получиться так:

private static const colors:Object = {
colors.blue=0x0000ff;
........
colors.yellowgreen=0x9acd32;
}

Разумеется, это неверно, и мы поиском и заменой (но не используя replace all) вначале удаляем строку "colors.", затем заменяем "=" на ":" и, в конце ";" на ",".

В итоге должно получиться так:

private static const colors:Object = {
blue:0x0000ff,
........
yellowgreen:0x9acd32
}

Теперь, нас этот объект не очень интересует и, если нажать волшебное сочетание клавиш SHIFT+CTRL+F, то всё перечисление из столбца превратится в одну строку и не будет занимать три экрана.

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

private function getAttribute(node:XMLNode, attributeName:String) : String {
return node.attributes[attributeName];
}

Процесс может оказаться утомительным и подверженым ошибкам. Чтобы этого избежать изучаем обращения к объекту attributes и видим, что строка кода всегда выглядит как "node.attributes.". Это дает нам возможность применить автозамену:
заменяем "node.attributes." (с точкой на конце!) на "getAttribute(node, ", в результате чего редактор расцветет массой ошибок. Это очень хорошо. Кликая на красные точки справа от скроллера мы переходим от ошибки к ошибке и ставим закрывающую скобку сразу после первого слова, следующего за "node". Заодно берем это слово в кавычки, поскольку это имя атрибута.

В процессе мы обнаруживаем еще одну прелесть рефакторинга: мы обнаружили ошибку несоответствия типов в строке:
hasRotate = getAttribute(node, "transform").indexOf("rotate");
Обратите внимание, что за внешне безобидным отсутствием типа пряталась куда более серьезная ошибка - несоответствие типа. Игнорирование малой ошибки может повлечь за собой большую.

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

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

Iv
12.03.2008, 14:32
Вначале проанализируем ошибку.
Переменная hasRotate объявлена в методе с типом Boolean, а в строке кода, которая дает ошибку, ей присваивается значение int.
Далее мы смотрим как используется эта переменная. Мы не будем доверять нашей внимательности, а пройдемся поиском по коду. Ищем вхождения слова "hasRotate" начиная со строки с ошибкой. Вхождение лишь одно и оно совсем рядом с ошибочной строкой. Это еще одна удача: изменения, которые сделаем будут локальны.
Смотрим, как используется переменная: с ее помощью производится проверка на наличие вхождений слова rotate.
В принципе, в таких случаях есть два пути: либо создание новой переменной и дальнейшая работа уже с ней, либо изменение кода таким образом, чтобы переменная использовалась правильно.
К первому решению чаще прибегают в случае, если изменение логики дальнейшей проверки нежелательно или затруднительно. Поскольку способ решения выбирается исходя из критерия минимального внесения изменений и у нас нет показаний за первое решение, поэтому прибегнем ко второму.
Продублируем две строки, начиная со строки с ошибкой, одну пару закомментируем и пока оставим на память. Вторую строку изменим.
Вот, что у меня получилось:
// hasRotate = getAttribute(node, "transform").indexOf("rotate");
// if (hasRotate > -1) {
hasRotate = getAttribute(node, "transform").indexOf("rotate") > -1;
if (hasRotate) {
Компилируем проект, убеждаемся в том, что и Flash компилятор к нашим изменениям отнесся благосклонно.

Обратите внимание на еще один момент: хотя логика вполне прозрачна, и мы могли бы вместо > -1 использовать, например != -1 или придумать что-нибудь еще, мы оставляем использованный автором способ, поскольку остальной код пока для нас - потемки и абсолютно быть уверенным в правоте своих действий невозможно.
По этой же причине мы оставляем закомментированные строки с исходным кодом.

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

Iv
12.03.2008, 17:17
Для того, чтобы решить что делать дальше, откройте в FDT панель Problems и отсортируйте по столбцу Resource.
Здесь мы видим два типа ошибок:
Local Variable is never used (локальная переменная никогда не используется)
Untyped member access (нетипизированный доступ к членам)

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

Просмотрев код видим, что ошибки вызваны нетипизированными обращениями к переменным объектов fill, stroke, firstP, lastP, lastC. Первые два объявлены как переменные класса, остальные как локальные переменные метода makeDrawCmds.

Начнем с локальных переменных. Судя по их использованию в коде, они могут быть типизированы как Point. Попробуем заменить тип на Point в их объявлении.
После замены типов и сохранения документа количество ошибок резко уменьшилось и изменился их тип на "You can not assign an 'Object' to a 'Point'".
Заменим, вот пример замены:
Было:
firstP = lastP = {x:Number(svgCmds[j]), y:Number(svgCmds[j + 1])};
Стало:
firstP = lastP = new Point(Number(svgCmds[j]), Number(svgCmds[j + 1]));

Чтобы ускорить этот процесс я воспользовался поиском и заменой, шаг за шагом заменяя строку "{x:" на "new Point(", внимательно следя за тем, чтобы не заменить эту строку там, где это не следовало бы делать. А именно в вызовах Math2.getQuadBez_RP(...).
Затем, кликая по образовавшимся у скроллера красным меткам ошибок, я переходил на очередную строку с ошибкой и заменял закрывающую фигурную скобку на обычную и удалял "y:".

В итоге, количество ошибок типизации в классе снизилось до шести. Все они - обращения к переменным объектов fill и stroke.
В отличие от предыдущего шага, у этих объектов отсутствует соответствие в родных классах Flash и нам потребуется создать собственные.

Сразу за самой последней закрывающей фигурной скобкой документа создадим внутренние классы.
Затем зададим им публичные поля, одноименные с теми, доступ к которым осуществляется из объектов fill и stroke. Добавим в конструкторы соответствующие аргументы.
Вот, что у меня получилось:
internal class Fill {

public var color:uint;
public var alpha:Number;

public function Fill(color:uint, alpha:Number) {
initInstance(color, alpha);
}

private function initInstance(color : uint, alpha : Number) : void {
this.color = color;
this.alpha = alpha;
}
}

internal class Stroke {

public var color:uint;
public var alpha:Number;
public var width:Number;

public function Stroke(color:uint, alpha:Number, width:Number) {
initInstance(color, alpha, width);
}

private function initInstance(color : uint, alpha : Number, width : Number) : void {
this.color = color;
this.alpha = alpha;
this.width = width;
}
}


Классы подготовлены, заменяем типы в объявлениях переменных fill и stroke на вновь созданные.
private var fill : Fill;
private var stroke : Stroke;
После этого список в панели Problems изменяется и вместо ошибок отсутствия типа мы получаем "You can not assign an 'Object' to an 'Stroke'".
Здесь способ ускорения процесса правки несколько иной. Перейдя на начало документа, используем сочетание клавиш CTRL+1. FDT - умница, он хоть и не может решить всех проблем, но он предлагает добавить кастинг соответствующего типа. Соглашаемся, после чего добавляем ключевое слово new и удаляем имена полей и фигурные скобки.
было:
fill = {color:0, alpha:0};
стало после кастинга:
fill = Fill({color:0, alpha:0});
мы исправили:
fill = new Fill(0, 0);

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

Закончив, выделим двойным кликом Fill проверяем, везде ли расставлено ключевое слово new. То же самое проделываем со Stroke.

В результате наших действий, в классе PathToArray не должно остаться ни одной подсвечивающейся ошибки.

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

Итоговый класс аттачу к этому посту.

Iv
12.03.2008, 18:04
Возможно, читатель думает, что я портировал проект, отрефакторил и оптимизировал и теперь, зная все подводные камни, веду их по спланированному сюжету.
Это не так.
Я не знаю, к чему мы придем. Я вначале делаю небольшой шаг, затем описываю что сделал. Всё по-честному.
- Что же мне дает уверенность в том, что в итоге всё получится хорошо?
- Ну, помимо природной наглости, я понимаю, что так или иначе удастся справиться с возникающими вопросами, а также я придерживаюсь простых правил, которые мне помогут:

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

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

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

Идем сверху вниз.

ratioTo
- пропускаем

intersect2Lines
- задаем всем аргументам тип Object
- задаем всем локальным переменным тип Number

rotation
- задаем всем аргументам тип Number

midPt
- задаем аргументам тип Number
- указываем возвращаемое значение Object

getQuadBez_RP
- здесь не указаны типы аргументов, для того, чтобы выяснить поиском по файлам (CTRL+H, File Search) ищем вызовы этого метода и, констатируя факт, указываем использованные типы:
public static function getQuadBez_RP(p1:Object, c1:Object, c2:Object, p2:Object, k:Number, qcurves:Array)
- задаем тип возвращаемого значения void
- задаем тип Number переменным dx и dy
- остальным переменным задаем тип Object

bezierSplit
- задаем всем аргументам тип Number
- задаем переменной m тип Function

pointOnCurve
- задаем всем аргументам тип Number
- видим, что можем легко избавиться от подсвечивания ошибки нетипизированного доступа и заменяем процесс создания объекта на такой:
public static function pointOnCurve(p1x:Number, p1y:Number, cx:Number, cy:Number, p2x:Number, p2y:Number, t:Number):Object {
return {x:p1x + t*(2*(1-t)*(cx-p1x) + t*(p2x - p1x)), y:p1y + t*(2*(1-t)*(cy-p1y) + t*(p2y - p1y))};
}

pointsOnCurve
- задаем всем аргументам тип Number

pointsOnLine
- задаем всем аргументам тип Number
- задаем переменной i тип int
- задаем всем остальным переменным тип Number

curveApproxLen
- задаем всем аргументам тип Number;

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

Iv
12.03.2008, 20:20
Следующим нашим шагом борьбы с нетипизированным доступом станет повсеместное удаление типа Object.

Изучив код, увидим, что очень часто Object используется там, где бы мог использоваться Point.
Порядок замены в таких случаях:

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

Процедура замены в каждом конкретном случае будет выглядеть следующим образом:

определяем требуемый тип, если нельзя применить стандартный, создаем собственный;
заменяем объект на типизированный;
исправляем ошибки, обнаруженные редактором;
тестируем;

Iv
12.03.2008, 22:22
Взглянув на код, определяем какие методы нам проще всего изменить. Внешним признаком будет служить нам то, что в качестве аргументов и возвращаемого значения не применяется тип Object.

Единственный подходящий метод - curveApproxLen.
Добавим в методе создание новой точки, назовем ее middle:
var middle:Point = new Point(mp.x, mp.y);
и далее заменим обращения к mp на middle.

Тут же видим: вызываемый метод pointOnCurve подходит для следующего шага, поскольку у него только возвращаемое значение имеет тип Object.
Заменяем объект и возвращаемый тип на Point:
public static function pointOnCurve(p1x:Number, p1y:Number, cx:Number, cy:Number, p2x:Number, p2y:Number, t:Number):Point {
return new Point(p1x + t*(2*(1-t)*(cx-p1x) + t*(p2x - p1x)), p1y + t*(2*(1-t)*(cy-p1y) + t*(p2y - p1y)));
}
и... ничего не произошло. Никаких новых ошибок не появилось. Причина в том, что использующие его объекты получали значение типа Object, а наш новый возвращаемый тип - Point является наследником Object.
Для того, чтобы найти места в коде, где используется метод pointOnCurve применим хитрость: переименуем pointOnCurve в pointOnCurve1, сохраним классс и по списку ошибок увидим где это.
Результат - два вызова в этом же классе.
Проще всего исправить curveApproxLen. Мы просто ставим вызов метода pointOnCurve вместо new Point(...) и удаляем строку объявления объекта mp.
Также можем удалить "Math2." в вызове.
В итоге должны получить вот такой метод:
public static function curveApproxLen(p1x:Number, p1y:Number, cx:Number, cy:Number, p2x:Number, p2y:Number):Number {
var middle:Point = pointOnCurve(p1x, p1y, cx, cy, p2x, p2y, 0.5);
var len1:Number = Math.sqrt((middle.x - p1x) * (middle.x - p1x) + (middle.y - p1y) * (middle.y - p1y));
var len2:Number = Math.sqrt((middle.x - p2x) * (middle.x - p2x) + (middle.y - p2y) * (middle.y - p2y));
return len1+len2;
}
Возвращаем методу pointOnCurve1 его первоначальное имя и делаем метод приватным: нигде коме этого класса он не используется.

Теперь можно разобраться со вторым методом, использующим pointOnCurve, это метод pointsOnCurve.
Сейчас он возвращает массив точек и... драма! Неприятная неожиданность: мы обнаруживаем, что это не могут быть объекты класса Point, поскольку в этом методе задается точке загадочная переменная r.

Ну, что-ж, это не критично. Cоздадим подкласс объекта Point.
Пока мы не знаем как его называть, поскольку сущность переменной r нам неизвестна. Мы только видим, что это угол, но угол чего именно на этом этапе мы выяснять не имеем права.
Второй вопрос - нам следует выяснить будут ли использованы эти точки вне класса Math2, чтобы определиться делать ли новый класс публичным или достаточно будет internal.
Для этого переименуем pointsOnCurve в pointsOnCurve1, сохраним документ и пройдемся по полученым ошибкам - местам вызова этого метода.
К моему удивлению - ни одной ошибки. Включаем занудство и используя поиск по файлам ищем вхождение строки "pointsOnCurve". Опять ничего кроме его самого.
Делаем вывод: мы нашли неиспользуемый метод. Тревога отменяется, закомментируем этот метод полностью и идем дальше.

Среди оставшихся четырех методов содержащих ошибки только bezierSplit в качестве параметров не требует Object, поэтому заглянем в него. Этот метод использует другой метод из этого же класса: midPt.
Переходим на него и видим, что возвращаемый тип можно заменить на Point, что и делаем. По прежней схеме с переименованием проверим, а не используется ли он где-то еще. Но в этот раз заодно зададим имя без сокращений: midpoint. А поскольку снаружи текущего класса этот метод нигде не используется, то сделаем его приватным. В итоге, он должен выглядеть так:
private static function midpoint(p1x : Number, p1y: Number, p2x: Number, p2y: Number):Point {
return new Point((p1x + p2x)/2, (p1y + p2y)/2);
}

После того, как в методе bezierSplit мы применим новое имя, на всякий случай протестируем проект.
Ошибок нет, можно продолжить доработку метода bezierSplit.
Заменим вызовы метода midpoint через локальную переменную m на обычный вызов и удалим строку объявления этой переменной. Это требуется для того, чтобы убедиться в правильности передаваемых аргументов.
Далее, всем объектам, которым присваивается возвращаемое методом midpoint значение зададим тип Point.
Но стоит ли на этом останавливаться? Если мы оставим тип Object переменным p1 и p2, то в возвращаемом объекте будет каша. Да, мы не знаем и пока не хотим знать логику приложения, но здравый смысл подсказывает, что p1 и p2 должны иметь такой-же как и у остальных тип - Point. Прислушаемся к голосу разума, исправим, и получим вот такой метод:
public static function bezierSplit(p1x:Number, p1y:Number, c1x:Number, c1y:Number, c2x:Number, c2y:Number, p2x:Number, p2y:Number):Object {
var p1:Point = new Point(p1x, p1y);
var p2:Point = new Point(p2x, p2y);

var p01:Point = midpoint (p1x, p1y, c1x, c1y);
var p12:Point = midpoint (c1x, c1y, c2x, c2y);
var p23:Point = midpoint (c2x, c2y, p2x, p2y);

var p02:Point = midpoint (p01.x, p01.y, p12.x, p12.y);
var p13:Point = midpoint (p12.x, p12.y, p23.x, p23.y);
var p03:Point = midpoint (p02.x, p02.y, p13.x, p13.y);

return { b0:{p1:p1, c1:p01, c2:p02, p2:p03}, b1:{p1:p03, c1:p13, c2:p23, p2:p2} };
}

И, раз уж мы произвели изменение от балды, тестируем проект.
Ошибок нет, идем дальше.

Я взялся за ratioTo, но переименовав его обнаружил, что он также нигде не используется. Проверил поиском - подтвердилось.
Закомментируем и идем дальше.

Iv
13.03.2008, 15:44
Выбирая из оставшихся двух переименованием, обнаруживаем, что метод intersect2Lines используется только в этом классе и лишь в одном месте. Делаем метод приватным.
Наиболее предсказуемые последствия возникнут если сейчас изменить возвращаемый тип. Логика подсказывает, что пересечение двух линий - точка. Заменяем возвращаемый тип на Point. Редактор тут же подсвечивает ошибки: возврат NaN и Object. В первом случае заменяем на null, во втором случае на new Point(...) - в трех местах.

После этого двойным кликом выделяем имя метода и используя CTRL+R открываем панель результатов поиска и там переходим на метод getgetQuadBez_RP и заменяем тип переменной s на Point. (Можно заодно удалить Math2. в вызове метода intersect2Lines)

Далее приведем в порядок аргументы метода intersect2Lines, задав им тип Point.

После этого в панели Problems отсортируем список по полю Description так, чтобы вверху оказались строки "You can not assign an 'Object' to an 'Point'" и приступим к исправлению этих ошибок.
Здесь нужно четко понимать: наша задача в данный момент - исправить только эти ошибки, а не весь проект. Наши исправления должны носить по возможности локальный характер.
Поэтому поступим следующим образом:
- переименуем первый аргумент в point1;
- перед вызовом intersect2Lines объявляем переменную p1 с типом Point и присваиваем ей новый объект Point, которому в качестве параметров передаем point1.x и point1.y;
- поступаем аналогично со следующим аргументом, задав ему имя control1.
- аналогично изменяем оставшиеся два аргумента, имеющие тип Object.
- сохраняем проект, тестируем.

Вот что имеем на данный момент:
public static function getQuadBez_RP(point1:Object, control1:Object, control2:Object, point2:Object, k:Number, qcurves:Array):void {
// find intersection between bezier arms
var p1:Point = new Point(point1.x, point1.y);
var c1:Point = new Point(control1.x, control1.y);
var c2:Point = new Point(control2.x, control2.y);
var p2:Point = new Point(point2.x, point2.y);
var s:Point = intersect2Lines (p1, c1, c2, p2);
// find distance between the midpoints
var dx:Number = (p1.x + p2.x + s.x * 4 - (c1.x + c2.x) * 3) * .125;
var dy:Number = (p1.y + p2.y + s.y * 4 - (c1.y + c2.y) * 3) * .125;
// split curve if the quadratic isn't close enough
if (dx*dx + dy*dy > k) {
var halves:Object = Math2.bezierSplit (p1.x, p1.y, c1.x, c1.y, c2.x, c2.y, p2.x, p2.y);
var b0:Object = halves.b0;
var b1:Object = halves.b1;
// recursive call to subdivide curve
getQuadBez_RP (p1, b0.c1, b0.c2, b0.p2, k, qcurves);
getQuadBez_RP (b1.p1, b1.c1, b1.c2, p2, k, qcurves);
} else {
// end recursion by saving points
qcurves.push({p1x:p1.x, p1y:p1.y, cx:s.x, cy:s.y, p2x:p2.x, p2y:p2.y});
}
}

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

Итак, в настоящий момент все ошибки типизации сконцентрировались в одном методе: getQuadBez_RP и разбиты на две группы: ошибки вызванные неверной типизацией аргументов и ошибки вызванные объектом halves. Следуя нашей стратегии вначале исправим ошибки, затрагивающие только методы текущего класса.


Объект halves - это возвращаемый методом bezierSplit объект, содержащий в себе два других объекта b0 и b1, каждый из которых содержит в себе по 4 объекта типа Point.
У нас есть выбор: либо создать пользовательский класс для типизации объекта halves, либо, поскольку его структура жестко задана, использовать массив и последующую типизацию.
В данном случае массив предпочтительнее, поскольку это более короткий и логичный путь. Название метода (bezierSplit - разделить безье) и использованное имя переменной, которой присваивается возвращаемое значение (halves - половинки), также говорят за использование массива. Ок, пойдем этим путем.

Проверим вначале на какие другие методы окажут воздействие изменения в bezierSplit. Сделать это можно либо переименовав его, либо с помощью CTRL+R.
Метод используется только в getQuadBez_RP. Это заодно нам позволяет сделать его приватным.

Далее:
- изменяем возвращаемый методом bezierSplit тип на Array;
- изменяем строку return на:
return [{p1:p1, c1:p01, c2:p02, p2:p03}, {p1:p03, c1:p13, c2:p23, p2:p2}];
- в методе getQuadBez_RP изменяем тип объекта halves на Array;
- изменяем присвоение переменным b0 и b1 на доступ к 0 и 1 элементам массива соответственно;
- тестируем.

Аналогичным образом поступаем с объектами в возвращаемом методом bezierSplit массиве - также делаем из них массивы:
return [[p1, p01, p02, p03], [p03, p13, p23, p2]];
- в методе getQuadBez_RP изменяем тип переменных b0 и b1 на Array;
- чтобы увидеть все вхождения b0 и b1, а также сделать код более читабельным, переименовываем эти переменные в bezier0 и bezier1 соответственно.
- дублируем строки, в которых появились ошибки, одну пару комментируем, чтобы не забыть что было изначально;
- в тех местах где подсветились ошибки заменяем имена переменных и доступ к их содержимому на оператор доступа к массиву. Замена производится соответственно: ".p1" на "[0]", ".c1" на "[1]", ".c2" на "[2]", ".p2" на "[3]".
Наример, было: b0.c1 стало: bezier0[1];

Результат должен получиться таким:
// getQuadBez_RP (p1, b0.c1, b0.c2, b0.p2, k, qcurves);
// getQuadBez_RP (b1.p1, b1.c1, b1.c2, p2, k, qcurves);
getQuadBez_RP (p1, bezier0[1], bezier0[2], bezier0[3], k, qcurves);
getQuadBez_RP(bezier1[0], bezier1[1], bezier1[2], p2, k, qcurves);

Тестируем. Всё в порядке? Тогда на этом первую половину процесса можно считать законченной.

Iv
13.03.2008, 16:11
Эту часть процесса мы будем делать исходя из очень простой логики действий: вначале вызываем ошибку, затем исправляем.

Изменяем первый тип аргумента метода getQuadBez_RP на тип Point. Сохраняем документ.
В панели Properties появляется список ошибок вида: "You can not assign an 'Object' to an 'Point'"
Двойным кликом переходим на первую ошибку и заменяем первый аргумент в вызове метода Math2.getQuadBez_RP с объекта на new Point(...).
Сохраняем документ, чтобы обновилась панель Problems.
Переходим к следующей ошибке этого типа.
Исправляем таким образом все ошибки данного типа.
Обязательно тестируем.

Результатом этого процесса должно стать полное отсутствие ошибок в панели Problems.
Это и было целью первого этапа рефакторинга. Мы ее добились и на этом этап можно считать законченным.

Iv
13.03.2008, 16:29
В процессе вы познакомились с различными приемами и принципами решения проблем.
Я и дальше постараюсь применять разные подходы к исправлению схожих задач, чтобы максимально охватить их спектр, даже, если в каком-то конкретном случае решение будет не самым оптимальным. В дальнейшем вы сами владея этими инстурументами сможете выбирать подходящий.


Подведем некоторые итоги.
Мы добивались того, чтобы в FDT не показывались ошибки. Но мы не прятали их, а именно исправляли. Результатом этого стало то, что кроме коллекции цветов в проекте не осталось объектов типа Object, а это - один из критериев оценки.
- Но устраивает ли нас код в текущем состоянии?
- Ни в коем случае.
Этот код нам непонятен, его повторное использование практически невозможно, внесение изменений в код крайне затруднительно.
Собственно именно поэтому мы и говорим об окончании первой волны рефакторинга, но это - только начало.

Iv
13.03.2008, 19:32
Перед началом следующего этапа почистим код: нужно отыскать и удалить неиспользуемые методы.

Раньше этого не стоило делать: мы пока не можем быть уверены в том, что в других частях проекта эти методы не использованы (имеется ввиду вторая часть - анимация отрисовки svg файла), а возвращаться в последствии к этапу приведения типов не хочется.

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

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

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

В итоге ненужные методы оказались только в классе Math2 и там, после удаления осталось их всего 4:
getQuadBez_RP - публичный, я его переместил в начало класса;
intersect2Lines - приватный;
midpoint - приватный;
bezierSplit - приватный;

Чтобы "сверить наши часы" я выкладываю файлы проекта в текущем состоянии.

Iv
13.03.2008, 20:45
Давайте определимся, с тем, чего мы будем добиваться на этом этапе и критерях его окончания.

Тот факт, что редактор не показывает нам ошибки нетипизированного доступа вовсе не означает, что в коде их нет.

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

К примеру, точку на плоскости в координатах x=10, y=15 можно представить разными способами:
var point1:Point = new Point(10, 15);
var point2:Array = new Array(10, 15);
var point3:Object = {x:10, y:15};
Но только первый способ применяет объект такого типа, который отражает сущность точки. В остальных случаях можно говорить о скрытом нетипизированном доступе.

Пройдемся по классам и отметим для себя такие случаи:

Класс SVGDisplayInFlash
метод getShapes:
- массивы хранят данные, а не объекты, представляющие сущности.

Класс Math2
метод getQuadBez_RP:
- в аргументах передаются точки вместо кривой Безье 3го порядка;
- внутри метода имеются обращения к массивам данных вместо обращений к сущностям.

метод intersect2Lines:
- в аргументах передаются точки вместо отрезков;

метод midpoint:
- в аргументах передаются координаты вместо отрезка;

метод bezierSplit:
- в аргументах передаются координаты вместо кривой Безье 3го порядка;
- возвращаемое значение содержит массивы данных вместо кривых Безье 3го порядка.

Класс PathToArray
метод makeDrawCmds:
- в аргументах и в теле метода широко используются массивы данных вместо классов, представляющих сущности.

Класс String2
случаев скрытого нетипизированного доступа нет.


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

Iv
13.03.2008, 21:31
Следуя нашим правилам, начинаем с методов, правка которых вызовет наименьшее влияние на проект.

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

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

Продублируем и закомментируем строку объявления метода. На память.
Заменим все аргументы метода на один (line:LineSVG).
Разумеется, такого типа не существует и, поэтому редактор подсветит ошибки.
Выделяем LineSVG и комбинацией CTRL+1 вызываем quick fix, соглашаемся на предложение создать класс LineSVG. Сохраняем его, возвращаемся.
Честно говоря, глядя на код хочется пойти по более короткому, хотя и теоретически опасному пути: немедленно перенести метод midpoint в класс LineSVG, поскольку он явно завистлив к данным этого класса и не использует данных текущего класса. Ну, что-ж, сократим путь. Переносим вместе с комментариями.

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

В итоге получаем такой класс:
package com.itechnica.svg {
import flash.geom.Point;

public class LineSVG {

private var startPoint : Point;
private var endPoint : Point;

public function LineSVG(start:Point, end : Point) {
initInstance(start, end);
}

private function initInstance(start : Point, end : Point) : void {
startPoint = start;
endPoint = end;
}

public function get start() : Point {
return startPoint;
}
public function set start(value : Point):void {
startPoint = value;
}
public function get end() : Point {
return endPoint;
}
public function set end(value : Point):void {
endPoint = value;
}


/**
* @returns Point the midpoint of current line segment
*/
public function midpoint():Point {
return Point.interpolate(startPoint, endPoint, 0.5);
}
}

}


Теперь можно приступить к исправлению образовавшихся ошибок в методе bezierSplit.

Iv
14.03.2008, 13:46
Исправим все строки, в которых применяется midpoint. Поступим следующим образом:
- продублируем и закомментируем верхнюю;
- удалим ошибочный вызов метода;
- вместо него впишем создание объекта LineSVG с пустыми new Point();
- из верхней строки скопируем пары координат и соответственно вставим в new Point()
- добавим вызов метода midpoint()

Результат должен получиться таким:
// var p01:Point = midpoint (p1x, p1y, c1x, c1y);
var p01:Point = new LineSVG(new Point(p1x, p1y), new Point(c1x, c1y)).midpoint();

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

Iv
14.03.2008, 15:59
В качестве аргументов методу bezierSplit передается четыре пары координат x и y, описывающих одну сущность - кривую Безье третьего порядка.
Чтобы описать эту сущность, создаем класс, в котором описываем базовые свойства кривой:

package com.itechnica.svg {
import flash.geom.Point;

public class CubicBezierSVG {

private var startPoint : Point;
private var startControlPoint : Point;
private var endControlPoint : Point;
private var endPoint : Point;

public function CubicBezierSVG(start:Point, startControl:Point, endControl:Point, end:Point) {
initInstance(start, startControl, endControl, end);
}

private function initInstance(start : Point, startControl : Point, endControl : Point, end : Point) : void {
startPoint = start;
startControlPoint = startControl;
endControlPoint = endControl;
endPoint = end;
}

public function get start() : Point {
return startPoint;
}
public function set start(value : Point):void {
startPoint = value;
}

public function get startControl() : Point {
return startControlPoint;
}
public function set startControl(value : Point):void {
startControlPoint = value;
}

public function get endControl() : Point {
return endControlPoint;
}
public function set endControl(value : Point):void {
endControlPoint = value;
}
public function get end() : Point {
return endPoint;
}
public function set end(value : Point):void {
endPoint = value;
}


}
}

И без специальных приборов видно, что если аргументы и возвращаемые значения метода bezierSplit класса Math2 заменить на объект CubicBezierSVG, то он будет завистлив к данным этого класса. В этот раз мы не будем шаг за шагом рефакторить метод, а с шашкой наголо создадим аналогичный метод в классе CubicBezierSVG.
Для этого просто скопируем метод в класс, продублируем и закомментируем верхний - на память.
Переименовываем метод в split, и делаем его публичным и не статическим. Удаляем аргументы. Редактор расцвел массой ошибок. Исправим их, заменив создание объектов Point из координат на соответствующие объекты Point текущего класса.
Для этого подглядываем в аргументы закомментированного класса: там точки идут по парам представляя контрольные точки кривой Безье.
Точки заменяем в том порядке, в котором они идут в аргументах закомментированного метода bezierSplit.
В итоге избавляемся от всех ошибок и видим, что можем удалить объявление точек p1 и p2 и заменить их непосредственно на startPoint и endPoint.
Для этого копируем startPoint в буфер обмена, удаляем строку инициализации p1, и там, где подсветилась ошибка вставляем из буфера обмена startPoint. Затем делаем то-же самое с p2.
Заменяем вложенные массивы на создание объектов CubicBezierSVG. После чего можем удалить закомментированный метод.

Затем мы можем заменить все new Point(...) на ранее полученные точки.

Полученный результат должен быть таким:

public function split():Array {
var p01:Point = new LineSVG(startPoint, startControlPoint).midpoint();
var p12:Point = new LineSVG(startControlPoint, endControlPoint).midpoint();
var p23:Point = new LineSVG(endControlPoint, endPoint).midpoint();

var p02:Point = new LineSVG(p01, p12).midpoint();
var p13:Point = new LineSVG(p12, p23).midpoint();
var p03:Point = new LineSVG(p02, p13).midpoint();

return [
new CubicBezierSVG(startPoint, p01, p02, p03),
new CubicBezierSVG(p03, p13, p23, endPoint)
];
}

Но на этом не остановимся и зададим читабельные имена и приведем объявления объектов в соответствие с их поведением:
public function split():Array {
const startMidpoint:Point = new LineSVG(startPoint, startControlPoint).midpoint();
const middleMidpoint:Point = new LineSVG(startControlPoint, endControlPoint).midpoint();
const endMidpoint:Point = new LineSVG(endControlPoint, endPoint).midpoint();

const startMiddleMidpoint:Point = new LineSVG(startMidpoint, middleMidpoint).midpoint();
const middleEndMidpoint:Point = new LineSVG(middleMidpoint, endMidpoint).midpoint();
const centerMidpoint:Point = new LineSVG(startMiddleMidpoint, middleEndMidpoint).midpoint();

return [
new CubicBezierSVG(startPoint, startMidpoint, startMiddleMidpoint, centerMidpoint),
new CubicBezierSVG(centerMidpoint, middleEndMidpoint, endMidpoint, endPoint)
];
}

Перейдем к замене использования метода Math2.bezierSplit на новый метод split класса CubicBezierSVG.

Iv
14.03.2008, 16:22
Прежде чем заменять вызовы старого метода на новый, мы обязательно(!!!) должны протестировать наш новый метод split.
Это ключевой и, как вы убедитесь, совсем не лишний шаг в выбранном способе рефакторинга.
Создавая новый метод мы имеем право делать всё что нам заблагорассудится с новым кодом, но старый код не имеем права трогать. И, когда новый метод готов, мы тестируем его. Тестирование нового метода обязательно производится заменой всей логики старого метода на вызов нового метода.

Продублируем метод Math2.bezierSplit и закомментируем верхний, чтобы было легко откатиться в случае ошибки.
После чего удалим всю старую логику и заменим на новую:
private static function bezierSplit(p1x:Number, p1y:Number, c1x:Number, c1y:Number, c2x:Number, c2y:Number, p2x:Number, p2y:Number):Array {
var curve : CubicBezierSVG = new CubicBezierSVG(
new Point(p1x, p1y),
new Point(c1x, c1y),
new Point(c2x, c2y),
new Point(p2x, p2y));

var halves:Array = curve.split();
var firstCurve : CubicBezierSVG = halves[0] as CubicBezierSVG;
var secondCurve : CubicBezierSVG = halves[1] as CubicBezierSVG;

return [
[firstCurve.start, firstCurve.startControl,
firstCurve.endControl, firstCurve.end],

[secondCurve.start, secondCurve.startControl,
secondCurve.endControl, secondCurve.end]
];
}

Тестируем. Работает! Только после этого мы можем приступать к следующему шагу: замене вызовов старого метода на новый.

Iv
14.03.2008, 17:07
Оказывается рановато заменять старые вызовы на новые.
Взглянув на метод Math2.getQuadBez_RP видим, что предварительно стоит заняться удалением локальных переменных p1, c1, c2, p2, которые дублируют точки, передаваемые в аргументах.
Для этого комментируем строку инициализации переменной и заменяем подсвечивающиеся ошибки на соответствующий аргумент. После чего удаляем ненужные закомментированные строки и получаем в итоге вот такой метод:
public static function getQuadBez_RP(point1:Point, control1:Point, control2:Point, point2:Point, k:Number, qcurves:Array):void {
// find intersection between bezier arms
var s:Point = intersect2Lines (point1, control1, control2, point2);
// find distance between the midpoints
var dx:Number = (point1.x + point2.x + s.x * 4 - (control1.x + control2.x) * 3) * .125;
var dy:Number = (point1.y + point2.y + s.y * 4 - (control1.y + control2.y) * 3) * .125;
// split curve if the quadratic isn't close enough
if (dx*dx + dy*dy > k) {
var halves:Array = bezierSplit (point1.x, point1.y, control1.x, control1.y, control2.x, control2.y, point2.x, point2.y);
var bezier0:Array = halves[0];
var bezier1:Array = halves[1];
// recursive call to subdivide curve
getQuadBez_RP (point1, bezier0[1], bezier0[2], bezier0[3], k, qcurves);
getQuadBez_RP(bezier1[0], bezier1[1], bezier1[2], point2, k, qcurves);
} else {
// end recursion by saving points
qcurves.push({p1x:point1.x, p1y:point1.y, cx:s.x, cy:s.y, p2x:point2.x, p2y:point2.y});
}
}

Iv
14.03.2008, 17:51
И вот, наконец можем заменить вызов метода Math2.bezierSplit на метод split класса CubicBezierSVG.

Дублируем весь код, находящийся внутри блока if (до строки с else) и закомментируем верхний.
Чтобы использовать метод split требуется экземпляр объекта CubicBezierSVG. Создаем его, заменяем метод со старого на новый и задаем новый тип переменным bezier0 и bezier1:
var sourceBezier : CubicBezierSVG = new CubicBezierSVG(point1, control1, control2, point2);
var halves:Array = sourceBezier.split();
var bezier0:CubicBezierSVG = halves[0] as CubicBezierSVG;
var bezier1:CubicBezierSVG = halves[1] as CubicBezierSVG;
Честно говоря, после этих действий я ожидал увидеть ошибки и исправить их. Но из-за примененных операторов доступа к массиву тип объектов не определяется и редактор не может обнаружить ошибки. Придется искать их другим способом.

Переименуем переменную bezier0 на firstHalf. Редактор кода подсвечивает ошибки и мы идем по ним, заменяя имя и обращение к точке, к которой осуществляется доступ.
Проделываем то-же самое с переменной bezier1, заменив ее имя на secondHalf.
Получаем в итоге:
getQuadBez_RP (point1, firstHalf.startControl, firstHalf.endControl, firstHalf.end, k, qcurves);
getQuadBez_RP(secondHalf.start, secondHalf.startControl, secondHalf.endControl, point2, k, qcurves);

Тестируем, убеждаемся в том, что всё в порядке.

Iv
14.03.2008, 20:53
Я напомню цель текущего шага: удаление случаев скрытого нетипизированного доступа.
Следующей мишенью наших действий станет метод Math2.intersect2Lines. Этот случай очень похож на предыдущий. Но, как я и обещал, чтобы разнообразить наш инструментарий способ рефакторинга будет выбран иной.

Вырежем и вставим метод в класс LineSVG, сделаем его публичным. Сохраним оба документа (CTRL+SHIFT+S). Используя панель Problems перейдем на строку с ошибкой и добавим "LineSVG." перед вызовом метода.

Поскольку мы намерены избавиться от скрытого нетипизированного доступа в аргументах метода, для начала снизим зависимость кода метода от аргументов.
В самом начале метода объявим два объекта first и second типа LineSVG и создадим их используя аргументы метода.
Везде далее в коде заменим использование аргументов на доступ через объекты LineSVG.
Теперь аргументы используются только для создания объектов LineSVG, чего мы и добивались.

Сделаем метод не статическим: удаляем static из объявления метода. Сохраняем документ, переходим на ошибку используя панель Problems. Создаем экземпляр LineSVG и от его имени вызываем метод intersect2Lines:
var startArm : LineSVG = new LineSVG(point1, control1);
var s:Point = startArm.intersect2Lines (point1, control1, control2, point2);
Поскольку данные startArm мы можем получить в классе LineSVG, удаляем два первых аргумента в вызове intersect2Lines. Затем переходим на метод и в нем также удаляем два первых аргумента. Подсвечиваются ошибки в объявлении first, удаляем эту строку. Затем удаляем все обращения к first, поскольку сейчас это текущий объект.
Заменим аргументы на один: second : LineSVG и удалим в теле метода объявление second.
Перейдем на ошибки в методе getQuadBez_RP.
Скопируем строку объявления startArm, переименуем переменную в endArm и скопируем параметры из вызова метода intersect2Lines. После этого параметры заменим на endArm.
В результате наших действий имя метода перестало отражать его суть. Переименуем в getLineIntersection и исправим вызов метода.
В итоге, измененная чать метода Math2.getQuadBez_RP будет выглядеть так:
var startArm : LineSVG = new LineSVG(point1, control1);
var endArm : LineSVG = new LineSVG(control2, point2);
var s:Point = startArm.getLineIntersection(endArm);
Имя аргумента метода getLineIntersection - second, больше не отражает сути. Переименуем в target.

Удалим объявления временных переменных ссылающихся на существующие значения.Подробно процесс на примере:
- закомментируем строку var x1 : Number = start.x;
- скопируем start.x в буфер обмена;
- выделим первую ошибку: обращение к x1;
- CTRL+F, ставим фокус в поле Replace With, CTRL+V, в нем должна появиться строка "start.x";
- отмечаем чек боксы Case Sensitive и Whole Word
- жмем Replace All
Впрочем, вам возможно будет удобнее заменить выделением и вставкой.

Далее также поступаем с переменными y1, x4, y4 и получаем такой код:
public function getLineIntersection(target : LineSVG) : Point {
var dx1 : Number = end.x - start.x;
var dx2 : Number = target.start.x - target.end.x;

if (!(dx1 || dx2)) return null;

var m1 : Number = (end.y - start.y) / dx1;
var m2 : Number = (target.start.y - target.end.y) / dx2;

if (!dx1) {
return new Point(start.x, m2 * (start.x - target.end.x) + target.end.y);
} else if (!dx2) {
return new Point(target.end.x, m1 * (target.end.x - start.x) + start.y);
}
var xInt : Number = (-m2 * target.end.x + target.end.y + m1 * start.x - start.y) / (m1 - m2);
var yInt : Number = m1 * (xInt - start.x) + start.y;
return new Point(xInt, yInt);
}

Теперь мы можем более осознанно подойти к именованию временных переменных.
Заменяем dx1 на currentDistanceX, а dx2 на targetDistanceX, исправляя подсвечивающиеся ошибки сразу после каждого переименования. Пробуем поставить const вместо var. Ошибок нет, так и оставим.

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

В вычислении переменных m1 и m2 видим, что используются вычисления, аналогичные currentDistanceX и targetDistanceX.
Объявим аналогичные локальные константы currentDistanceY и targetDistanceY и присвоим им соответствующие значения скопировав из вычислений. Затем заменим вычисления на эти константы:
const currentDistanceY : Number = end.y - start.y;
const targetDistanceY : Number = target.start.y - target.end.y;

var m1 : Number = currentDistanceY / currentDistanceX;
var m2 : Number = targetDistanceY / targetDistanceX;
Значение переменных стало куда очевиднее: это отношение высоты к ширине габаритного прямоугольника отрезков. Отношение высоты к ширине это тангенс. Переименовываем и делаем константами.
const currentTangent : Number = currentDistanceY / currentDistanceX;
const targetTangent : Number = targetDistanceY / targetDistanceX;
Дальше по коды мы только переименуем переменные xInt и yInt на intersectionX и intersectionY и сделаем их константами. Остановимся на этом, ибо нет предела совершенству.
В итоге имеем вот такой метод:
public function getLineIntersection(target : LineSVG) : Point {
const currentDistanceX : Number = end.x - start.x;
const targetDistanceX : Number = target.start.x - target.end.x;

if (!(currentDistanceX || targetDistanceX)) {
return null;
}
const currentDistanceY : Number = end.y - start.y;
const targetDistanceY : Number = target.start.y - target.end.y;

const currentTangent : Number = currentDistanceY / currentDistanceX;
const targetTangent : Number = targetDistanceY / targetDistanceX;

if (!currentDistanceX) {
return new Point(start.x, targetTangent * (start.x - target.end.x) + target.end.y);
} else if (!targetDistanceX) {
return new Point(target.end.x, currentTangent * (target.end.x - start.x) + start.y);
}
const intersectionX : Number = (-targetTangent * target.end.x + target.end.y + currentTangent * start.x - start.y) / (currentTangent - targetTangent);
const intersectionY : Number = currentTangent * (intersectionX - start.x) + start.y;
return new Point(intersectionX, intersectionY);
}

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

А если серьезно, то обратите внимание на то, что мы уже довольно давно работаем с кодом, но до сих пор так и не вникали в суть его логики. Более того, если вы обратили внимание, в процессе рефакторинга код понемногу сам нам рассказывает о себе.

Iv
14.03.2008, 21:48
Вернемся в класс Math2.

Здесь мы видим сиротливо приютившийся метод getQuadBez_RP и мы понимаем, что из-за его завистливости к данным других объектов и вселенского одиночества в классе, он обречен на перемещение, а класс будет удален.

Поскольку в первую очередь мы намерены избавить метод от скрытого нетипизированного доступа в аргументах, то видим, что заменяться они будут на объект CubicBezierSVG. Туда и будем перемещать.
Правда, мы видим, что метод также использует класс LineSVG и, вполне возможно, что он будет завистлив к данным этого класса. Но это не проблема. Мы впоследствии сможем переместить, если потребуется.

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

Возвращаемся к методу getQuadBez_RP.
Как и в предыдущем случае, нам вначале потребуется избавиться от зависимости кода от аргументов метода, которые мы будем заменять на объект CubicBezierSVG.
Для этого вначале метода создаем экземпляр класса CubicBezierSVG:
const source:CubicBezierSVG = new CubicBezierSVG(point1, control1, control2, point2);
И везде в коде метода вместо обращения к аргументам используем доступ через объект source.
Чтобы гарантировать себя от невнимательности, действуем по простой схеме:
- переименовываем аргумент;
- новое имя используем только в инициализации объекта source;
- остальные обращения заменяем на доступ через объект source.

Вот что в итоге получилось:
public static function getQuadBez_RP(start:Point, startControl:Point, endControl:Point, end:Point, k:Number, qcurves:Array):void {
const source:CubicBezierSVG = new CubicBezierSVG(start, startControl, endControl, end);
// 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 sourceBezier : CubicBezierSVG = new CubicBezierSVG(source.start, source.startControl, source.endControl, source.end);
var halves:Array = sourceBezier.split();

var firstHalf:CubicBezierSVG = halves[0] as CubicBezierSVG;
var secondHalf:CubicBezierSVG = halves[1] as CubicBezierSVG;
// recursive call to subdivide curve
getQuadBez_RP (source.start, firstHalf.startControl, firstHalf.endControl, firstHalf.end, k, qcurves);
getQuadBez_RP(secondHalf.start, secondHalf.startControl, secondHalf.endControl, source.end, 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});
}
}

Следующий шаг, как вы помните, переход от статичного метода к методу экземпляра класса.
Удаляем ключевое слово static, сохраняемся и идем исправлять ошибки.
Принцип, как всегда прост:
- вначале метода объявляем переменную: var source:CubicBezierSVG;
- в коде, перед ошибочной строкой создаем экземпляр класса CubicBezierSVG:
source = new CubicBezierSVG();
- копируем первые четыре аргумента метода getQuadBez_RP в аргументы конструктора CubicBezierSVG.
- заменяем обращение к статическому методу на обращение через экземпляр source.
- переходим к следующей ошибке, пока они не иссякнут.
- тестируем проект.
- не забываем радоваться тому, как у нас ловко всё получается.

Iv
15.03.2008, 16:04
А теперь можно заняться удалением теперь уже ненужных аргументов.

Заменим создание объекта source на присвоение ему this:
const source:CubicBezierSVG = this;
И тут же тестируем и видим, что происходит переполнение стека. Причина совсем рядом: рекурсивный вызов метода.
До тех пор, пока использовались аргументы, в методе создавались новые объекты CubicBezierSVG. А как только перешли на использование this наша невнимательность выползла наружу: мы не добавили объекты, от имени которых рекурсивно вызывается метод getQuadBez_RP.
Исправим.
Для начала откатимся назад до рабочего состояния:
const source:CubicBezierSVG = new CubicBezierSVG(start, startControl, endControl, end);
Вот тут-то и проявляется прелесть исправления кода короткими шагами.
Протеституем, убедимся, что всё работает.

Вот участок кода, который требует вмешательства:
getQuadBez_RP (source.start, firstHalf.startControl, firstHalf.endControl, firstHalf.end, k, qcurves);
getQuadBez_RP(secondHalf.start, secondHalf.startControl, secondHalf.endControl, source.end, k, qcurves);
Мы видим, что аргументы каждого вызова берутся из разных объектов. Для вызова в основном используются данные половинок кривой sourceBezier, а отличаются от остальных в первом случае - первый аргумент, а во втором случае четвертый и оба берут данные из совсем другого объекта - source. Но такие ли разные объект source и sourceBezier? Мы видим, что вовсе нет. Объект sourceBezier создан полностью из данных объекта source. Так может попробовать удалить sourceBezier и использовать source вместо него?

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

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

Вернемся к коду и оценим риски.
Использование объекта 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
15.03.2008, 20:50
Краткое содержание предыдущей серии:
мы хотим вместо 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
15.03.2008, 22:20
Как мы помним, наша попытка заменить создание нового объекта на использование 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
15.03.2008, 22:52
Загадочная буква 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
15.03.2008, 23:42
Если объект создавался, значит, что его кто-то использовал. И это был нетипизированый доступ. Включаем отдел головного мозга, ответственный за Шерлока Холмса и проводим частное расследование. Задача - выяснить кто использует массив, передаваемый в аргументах.

Выделяем имя метода 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
16.03.2008, 00:07
Краткое отступление: вы не думайте, что код, написанный Helen плох. Для своего времени это прекрасный код.
Просто в то время цели, которых добивались от кода, были совсем другими. Никто даже не думал о том, чтобы код был простым и читабельным. Более того, лучшим считался тот программист, который был способен написать много непонятного кода и, при этом, умудриться самому в нем не запутаться.
Еще один очень вероятный фактор: Helen привела к такому виду код в процессе оптимизации, а мы сейчас идем в обратном направлении.

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

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

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

Iv
16.03.2008, 01:32
В классе 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
16.03.2008, 02:23
Продолжим приводить в порядок наш новый метод.
Организуем типизированный доступ к данным:
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
16.03.2008, 07:37
Скопируем "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
16.03.2008, 18:59
Общий вид фабричного метода получается таким:
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;
// }
}
}

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

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

Iv
16.03.2008, 19:32
Создаем классы команд, все они наследуются от DrawingCommand.
Вот как будут выглядеть конструкторы этих классов:
public function FillCommand (color:uint, alpha:Number) {...}
public function StyleCommand (thickness:Number, color:uint, alpha:Number) {...}
public function MoveCommand (target:Point) {...}
public function LineCommand (target:Point) {...}
public function CurveCommand (control:Point, end:Point) {...}

Добавим во все классы инициализацию экземпляров и методы доступа к данным.

И, раз уж мы принялись структурировать проект, в папке svg создадим папку geom и переместим туда классы CubicBezierSVG, LineSVG и QuadraticBezierSVG. Правим ошибки, тестируем.

Iv
16.03.2008, 21:57
Возвращаемся в класс PathToArray и оцениваем возможность применения новых классов.
Только в этот момент обнаруживаю, что замена строковых данных на константы не везде сработала, поскольку строковые данные использовались не только в двойных кавычках, но и в одинарных.
Производим замену на константы.

Наша задача - заменить в массиве нетипизированные данные на объекты данных. Это нужно сделать как в процедуре наполнения массива, так и в процедуре использования.
Массив наполняется в классе PathToArray, где называется "drawCmds" и используется в классе SVGDisplayInFlash, где называется "dCmds". Заменим эти имена на одно: drawingCommands.

Начнем замену на объекты данных с класса PathToArray. Благодаря тому, что строки заменены на константы, нам трудно ошибиться какого класса команда должна быть использована.
Дублируем старую команду, комментируем верхнюю, а в нижней делаем соответствующую замену. Первые три замены будут выглядеть так:
// drawingCommands.push([DrawingCommand.FILL, [fill.color, fill.alpha]]);
drawingCommands.push(new FillCommand(fill.color, fill.alpha));

// drawingCommands.push([DrawingCommand.STYLE, [stroke.width, stroke.color, stroke.alpha]]);
drawingCommands.push(new StyleCommand(stroke.width, stroke.color, stroke.alpha));

// drawingCommands.push([DrawingCommand.MOVE, [firstP.x, firstP.y]]);
drawingCommands.push(new MoveCommand(firstP.clone()));
Обратите внимание на то, что в последнем случае примера в качестве аргумента конструктора мы использовали копию точки, а не ее саму.
Заменяем все случаи добавления в массив и переходим к методу getShapes класса SVGDisplayInFlash.
Здесь для замены мы используем наши новые возможности. Мы можем заменить условную логику на полиморфизм. Суть будет заключаться в том, что в каждом классе команды рисования мы создадим метод draw и будем вызывать его в цикле.
Чтобы реализовать такое поведение можно пойти двумя путями: создать метод draw в классе DrawingCommand, а в подклассах его перекрыть, либо создать интерфейс и в классах рисования имплементировать его.
Мы пойдем вторым путем, поскольку нам это поможет создавать сами методы. К тому-же нам не придется беспокоиться о том, что кому-то в голову придет использовать метод draw у экземпляра класса DrawingCommand.

В папке draw cоздаем интерфейс IDrawable и описываем метод draw:
package com.itechnica.svg.draw {
import flash.display.Graphics;

public interface IDrawable {
function draw(target:Graphics):void;
}
}

Затем классам папки draw, кроме DrawingCommand, добавим имплементацию интерфейса, например:
public class CurveCommand extends DrawingCommand implements IDrawable {...
После этого редактор подсветит ошибку, используя CTRL+1 исправляем ее - добавится метод draw. Каждому классу задаем свою соответствующую логику этого метода, например, класс CurveCommand будет использовать curveTo:
public function draw(target:Graphics):void {
target.curveTo(controlPoint.x, controlPoint.y, end.x, end.y);
}
Аналогично поступаем с остальными классами.

После чего вся логика, ранее реализованая в switch заменится на две строки:
private function getShapes(iPath:Number):void {
var drawingCommands:Array = new Array();
conv = new PathToArray(paths[iPath], drawingCommands);

// draw the shapes in clips in holder movieclip
// holder.createEmptyMovieClip("p"+iPath, iPath+1);

const drawLayer:Sprite = new Sprite();
holderMc.addChild(drawLayer);
const drawTarget:Graphics = drawLayer.graphics;

for (var i:Number=0; i<drawingCommands.length; i++) {
var command:IDrawable = drawingCommands[i] as IDrawable;
command.draw(drawTarget);
}

// repeat til all paths have been read
if (++iPath < nPathNodes) getShapes(iPath);
}
Тестируем, всё работает.

Iv
17.03.2008, 14:24
В раздумиях великих
по коду класса я брожу.
PathToArray.

Iv
17.03.2008, 15:01
Следующая цель у меня сомнений не вызывает. Мне категорически не нравятся два огромных метода в классе PathToArray. Они огромны, из-за этого их логика сложна для восприятия.
Вспомните, каким был метод getShapes и каким он стал. В итоге хочется чего-то такого.
Плюс к этому, не хочется повторяться: я помню, что статья учебная и главная цель научиться чему-то новому и полезному.

Смотрим, что из себя структурно представляет метод makeDrawCmds. В цикле while имеется switch, в каждом из case-ов которого имеется логика конвертации набора данных в объект данных.
Всё бы ничего, но у нас в case-ах имеются загадочные строковые данные, которые нужно сделать константами. Мы понимаем откуда они вообще взялись: они используются в svg для обозначения того, какие действия следует произвести с данными.
Но это в общем. А вот что именно обозначают эти буквы мы не знаем. Поэтому осмысленные имена дать не можем.
Чтобы разобраться, обратимся к первоисточнику: описанию формата SVG: http://www.w3.org/Graphics/SVG/

Iv
17.03.2008, 15:52
Смотрим и обнаруживаем раздел SVG in Russian. В словарик заглядывать не надо, чтобы понять что это за линки.
Заглянем для общего развития на википедию: http://ru.wikipedia.org/wiki/SVG
Погуляв по русским линкам я нашел:
http://jre.cplire.ru/koi/oct01/5/text.html
Для рисования линий в SVG используется тег <path>. Здесь атрибут d представляет параметры кривой – буквы обозначают команды, а цифры параметры этой команды. Например, M – это команда MoveTo, а цифры, следующие за ней, обозначают координаты, куда нужно перейти, чтобы начать рисовать кривую. Аналогично, С – это команда CurveTo, которая рисует кривую Безье.
http://www.xml.nsu.ru/extra/svgintro_2.xml
Траектории
Траектории в SVG представляют контуры объектов. Одним из наиболее важных атрибутов элемента <path> является атрибут d, который содержит данные, описывающие траекторию. Атрибут d несет инструкции типа "moveto" (переместить), "line" (провести линию), "curve" (провести кривую Безье второй или третьей степени), "arc" (провести дугу) и "closepath" (закрыть траекторию). Все эти инструкции обозначаются какой-нибудь одной буквой (например, "moveto" обозначается символом M). В нашем примере мы используем инструкции "moveto" и "quadratic bezier curve". Следующий код задает кривую, изображающую телефонный шнур:

<path id="cord" d="M 235,130 q 25,0 25,25 q 0,25 25,25 q 25,0 25,
25 q 0,25 25,25 q 25,0 25,25 q 0,25 25,25 q 25,0 25,25 q 0,
25 25,25 q 25,0 25,25" />

В атрибуте d мы сначала задаем инструкцию M, которая дает указание сделать сдвиг к новой точке, от нее начнется кривая. Заглавная M в данном случае означает, что в описании используются абсолютные координаты, а маленькая m - что используются относительные. После того, как мы с помощью инструкции "moveto" определили начальную точку траектории, мы используем инструкцию q для задания сегмента кривой Безье второго порядка. И снова, маленькая q означает, что описание дается в относительных координатах, а заглавная Q - что в абсолютных. Параметрами команды q является серия координатных пар в виде (x1,y1 x,y). Эти координатные пары задают кривую Безье второго порядка, которая проходит из текущей точки в точку с координатами (x,y) используя точку (x1,y1) в качестве контрольной. После выполнения одной инструкции q, текущая точка переместится в соответствии с координатным параметром. Поскольку мы используем относительные координаты, мы можем переместить изображение телефонного шнура, просто изменив координаты начальной точки.
Эта информация помогает нам разобраться, то для дальнейших действий мы используем всё-таки официальное описание: http://www.w3.org/TR/SVG11/paths.html

Iv
17.03.2008, 16:19
Суммируем наши знания в коде: создадим класс FormatSVG и перенесем в него из описания формата все инструкции атрибута d:
package com.itechnica.svg {

// http://www.w3.org/TR/SVG11/paths.html
public class FormatSVG {

// 8.3.2 The "moveto" commands

public static const MOVE_TO_ABSOLUTE : String = "M";
public static const MOVE_TO_RELATIVE : String = "m";

// 8.3.3 The "closepath" command
public static const CLOSEPATH_ABSOLUTE : String = "Z";
public static const CLOSEPATH_RELATIVE : String = "z";

// 8.3.4 The "lineto" commands
public static const LINE_TO_ABSOLUTE : String = "L";
public static const LINE_TO_RELATIVE : String = "l";

public static const HORIZONTAL_LINE_TO_ABSOLUTE : String = "H";
public static const HORIZONTAL_LINE_TO_RELATIVE : String = "h";

public static const VERTICAL_LINE_TO_ABSOLUTE : String = "V";
public static const VERTICAL_LINE_TO_RELATIVE : String = "v";

// 8.3.6 The cubic Bйzier curve commands
public static const CUBIC_CURVE_TO_ABSOLUTE : String = "C";
public static const CUBIC_CURVE_TO_RELATIVE : String = "c";

public static const CUBIC_SMOOTH_CURVE_TO_ABSOLUTE : String = "S";
public static const CUBIC_SMOOTH_CURVE_TO_RELATIVE : String = "s";

// 8.3.7 The quadratic Bйzier curve commands
public static const QUADRATIC_CURVE_TO_ABSOLUTE : String = "Q";
public static const QUADRATIC_CURVE_TO_RELATIVE : String = "q";

public static const QUADRATIC_SMOOTH_CURVE_TO_ABSOLUTE : String = "T";
public static const QUADRATIC_SMOOTH_CURVE_TO_RELATIVE : String = "t";

// 8.3.8 The elliptical arc curve commands
public static const ARC_TO_ABSOLUTE : String = "A";
public static const ARC_TO_RELATIVE : String = "a";
}
}

Iv
17.03.2008, 21:46
Перед тем, как начать замену на константы, сделаем один ход, который и впоследствии нам пригодится.
Мы сделаем простой тест.
Задача теста будет проста: сверять результаты работы скрипта с некими образцами.

Для начала создадим методы toString в классах папки draw:

DrawingCommand:
public function toString() : String {
throw new Error("com.itechnica.svg.draw.DrawingCommand");
return "com.itechnica.svg.draw.DrawingCommand";
}
CurveCommand:
override public function toString() : String {
return "curveTo(" + controlPoint.x + ", " + controlPoint.y + ", " + end.x + ", " + end.y + ")";
}
FillCommand:
override public function toString() : String {
return "beginFill(0x"+colorValue.toString(16)+", "+ alphaValue+")";
}
LineCommand:
override public function toString() : String {
return "lineTo(" + targetPoint.x + ", " + targetPoint.y + ")";
}
MoveCommand:
override public function toString() : String {
return "moveTo(" + targetPoint.x + ", " + targetPoint.y + ")";
}
StyleCommand:
override public function toString() : String {
return "beginFill("+thicknessValue+", 0x"+colorValue.toString(16)+", "+ alphaValue+")";
}

Затем создадим класс Test:
package {

public class Test {

public static const testArray:Array = [];

}
}

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

private function getShapes(iPath:Number):void {
var drawingCommands:Array = new Array();
conv = new PathToArray(paths[iPath], drawingCommands);

// TEST as3
const updateTestArray:Boolean = false;

const testString:String = Test.testArray[iPath];
const currentString:String = drawingCommands.toString();

if (updateTestArray) {
trace("[\""+currentString+"\"],");
} else if (currentString != testString) {
throw new Error("SVGDisplayInFlash.getShapes test error");
}
// END TEST as3
................

Компилируем проект и получаем в окне Output длинную строку. Копируем и вставляем ее в массив testArray класса Test.
После этого заменяем значение updateTestArray на false и компилируем еще раз.
Если ошибок не возникло, значит тест пройден успешно.

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

Iv
17.03.2008, 21:53
Вернемся к константам класса FormatSVG.
Ранее в классе DrawingCommand мы уже использовали константы, описывающие эти же сущности. Пришел их черед - они отслужили своё. По очереди закомментируем их, а в местах использования заменим на соответствующие из нашего нового класса. Каждый раз тестируем проект.

По ходу дела обнаруживаем, что неверно интерпретировали "S" - думали, что это стиль, а оказалось что это кубическая кривая. Всё-таки работа с первоисточником очень важна и к описанию формата SVG надо было обращаться раньше.

Следующим шагом избавляемся от использования строковых данных в case-ах метода makeDrawCmds.
Это занудная работа, поэтому выложу результат.

Iv
18.03.2008, 18:22
Теперь мы готовы к следующему шагу: выделению методов из makeDrawCmds.
Начнем с самого первого case: FormatSVG.MOVE_TO_ABSOLUTE.
- копируем содержимое case до строки break.
- создаем метод createMoveToCommand
- вставляем скопированное в метод.
- задаем аргументы, одноименные с переменными, вызывающими ошибку.
Получаем такой метод:

private function createMoveToCommand(firstP:Point, lastP:Point, svgCmds:Array, j:int) : void {
// moveTo point
firstP = lastP = new Point(Number(svgCmds[j]), Number(svgCmds[j + 1]));
drawingCommands.push(new FillCommand(fill.color, fill.alpha));
drawingCommands.push(new StyleCommand(stroke.width, stroke.color, stroke.alpha));
drawingCommands.push(new MoveCommand(firstP.clone()));

j += 2;
if (j < svgCmds.length && !isNaN(Number(svgCmds[j]))) {
do {
// if multiple points listed, add the rest as lineTo points
lastP = new Point(Number(svgCmds[j]), Number(svgCmds[j + 1]));

drawingCommands.push(new LineCommand(lastP.clone()));

firstP = lastP;
j += 2;
} while (j < svgCmds.length && !isNaN(Number(svgCmds[j])));
}
}
Тестируем, получаем ошибку, разбираемся в чем дело.
Видим, что в методе изменяется значение переменной j, которая в дальнейшем используется в коде. Добавляем возвращаемое значение:

private function createMoveToCommand(firstP:Point, lastP:Point, svgCmds:Array, j:int) : int {
.........
return j;
}
И в месте вызова добавляем присвоение нового значения j:
j = createMoveToCommand(firstP, lastP, svgCmds, j);
Тестируем, получаем ошибку.

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

Еще раз проверил старый код: закомментировал вызов метода, раскомментировал код, протестировал, всё работает.
Возвращаем комментарии обратно и смотрим, что еще может сносить.
Вполне реально - переприсвоение firstP и lastP. Встречается в четырех строках метода. И, если первую строку мы в состоянии вынести за пределы метода, то остальные две не получится.
Попробуем решить проблему так:
- вынесем объявление точек из метод в класс, сделаем переменные приватными.
private var firstP:Point;
private var lastP:Point;
private var lastC:Point;
- добавим в начало метода обнуление переменных
firstP = lastP = lastC = null;
- удалим первые два аргумента в вызове метода и в методе.
Тестируем. Работает.
Становится понятна причина: в аргументах мы объявили точки, и присвоение новых значений переменным, объявленным в аргументах не давало требуемой замены одноименных переменных в методе makeDrawCmds.

Копируем вызов:
j = createMoveToCommand(svgCmds, j);
переходим на case FormatSVG.LINE_TO_RELATIVE и вставляем. Переименовываем в createLineToRelativeCommand. После чего с помощью CTRL+1 создаем метод.
Копируем и комментируем дальнейший код до break. Переходим на новый метод и вставляем в него скопированный код. Заменяем return null на return j. Тестируем. Работает.
Аналогично двигаемся дальше. После каждого шага тестируем.

Первой остановкой на этом пути стало появление сubicBezier. Поиском по коду смотрим каким образом она применяется. В коде нет случаев, когда сubicBezier присваивается другой переменной. Попробуем объявить ее локальной переменной.
И не забываем изменить возвращаемое значение с null на j. Тестируем, всё ок. Дальше планируем в аналогичных случаях поступать также.
Если по ходу дела нарываемся на бесконечный цикл, значит попросту забыли заменить null на return j. Нарывался два раза.

После того, как закончили и протестировали, сносим закомментированные строки и ненужное более объявление переменной сubicBezier.
В итоге должен получиться вот такой метод:
private function makeDrawCmds(svgCmds:Array):void {
var j:Number = 0;
var cmd:String;
firstP = lastP = lastC = null;
do {
cmd = svgCmds[j++];
switch (cmd) {
case FormatSVG.MOVE_TO_ABSOLUTE :
j = createMoveToCommand(svgCmds, j);
break;
case FormatSVG.LINE_TO_RELATIVE :
j = createLineToRelativeCommand(svgCmds, j);
break;

case FormatSVG.LINE_TO_ABSOLUTE :
j = createLineToCommand(svgCmds, j);
break;

case FormatSVG.HORIZONTAL_LINE_TO_RELATIVE :
j = createHorizontalLineToRelativeCommand(svgCmds, j);
break;

case FormatSVG.HORIZONTAL_LINE_TO_ABSOLUTE :
j = createHorizontalLineToCommand(svgCmds, j);
break;

case FormatSVG.VERTICAL_LINE_TO_RELATIVE :
j = createVerticalLineToRelativeCommand(svgCmds, j);
break;

case FormatSVG.VERTICAL_LINE_TO_ABSOLUTE :
j = createVerticalLineToCommand(svgCmds, j);
break;

case FormatSVG.QUADRATIC_CURVE_TO_RELATIVE :

j = createQuadraticCurveToRelativeCommand(svgCmds, j);
break;

case FormatSVG.QUADRATIC_CURVE_TO_ABSOLUTE :
j = createQuadraticCurveToCommand(svgCmds, j);
break;

case FormatSVG.CUBIC_CURVE_TO_RELATIVE :
j = createCubicCurveToRelativeCommand(svgCmds, j);
break;

case FormatSVG.CUBIC_CURVE_TO_ABSOLUTE :
j = createCubicCurveToCommand(svgCmds, j);
break;

case FormatSVG.CUBIC_SMOOTH_CURVE_TO_RELATIVE :
j = createCubicSmoothCurveToRelativeCommand(svgCmds, j);
break;

case FormatSVG.CUBIC_SMOOTH_CURVE_TO_ABSOLUTE:
j = createCubicSmoothCurveToCommand(svgCmds, j);
break;

case FormatSVG.CLOSEPATH_RELATIVE :
case FormatSVG.CLOSEPATH_ABSOLUTE :
j = createClosePathCommand(svgCmds, j);
break;
} // end switch
} while (j < svgCmds.length);
}
Не фонтан, конечно, но это уже кое-что: мы в состоянии увидеть логику метода.

Iv
18.03.2008, 20:35
Раз уж взялись за этот метод, доведем дело до конца.
Этот switch мне очень не нравится. Что мы можем сделать, чтобы избавиться? Есть два пути: первый мы уже использовали в похожей ситуации: заменили условный выбор полиморфизмом в методе getShapes класса SVGDisplayInFlash.
Подойдет ли аналогичный способ для этого случая? Теоретически, мы можем загнать код в такие рамки. Но это неудобно и потребует существенного изменения логики приложения, чего мы избегаем на данном этапе.
Второй путь - применить пространства имен, его и попробуем применить.

Делаем небольшой тест:
- для начала создадим пространство имен
private namespace MOVE_TO_ABSOLUTE = "M";
затем заменим пространство имен private у метода createMoveToCommand на созданное:
MOVE_TO_ABSOLUTE function createMoveToCommand(svgCmds:Array, j:int) : int {...

И создадим тестовый код в соответствующем case:
case FormatSVG.MOVE_TO_ABSOLUTE :

var createMoveToCommand:*;
var space:Namespace = Namespace(cmd);
var method:Function = space::createMoveToCommand as Function;
trace(space+ " >>> "+method);

j = method(svgCmds, j);
break;
Протестируем, всё работает.

Идея заключается в том, чтобы метод makeDrawCmds принял примерно такой вид:
private function makeDrawCmds(svgCmds:Array):void {

firstP = lastP = lastC = null;
var j:Number = 0;
var createDrawCommand:Function;
var space:Namespace;
var method:Function;

do {
space = Namespace(svgCmds[j++]);
method = space::createDrawCommand as Function;
j = method(svgCmds, j);

} while (j < svgCmds.length);
}
Для этого придется создать создать массу пространств имен и перенести в эти пространства имен методы create...Command и переименовать их все в createDrawCommand.
В итоге мы получим вполне симпатичный метод makeDrawCmds и... несколько неприятностей вдовесок.

Для объявления пространства имен мы не можем использовать константы, объявленные в классе FormatSVG. Мы должны обязательно их объявлять вот так:
private namespace MOVE_TO_ABSOLUTE = "M";
но не можем применить ни один их этих вариантов:
private namespace MOVE_TO_ABSOLUTE = new Namespace(FormatSVG.MOVE_TO_ABSOLUTE);
private namespace MOVE_TO_ABSOLUTE = FormatSVG.MOVE_TO_ABSOLUTE;
Это значит, что если вдруг нам понадобится изменить значение константы, то придется не забыть и изменить объявление пространства имен. Мы-то это знаем, но не те, кто будут править код после нас.

Второй неприятный довесок: FDT плохо поддерживает работу с пространствами имен. Например, чтобы избавиться от подсвечивания ошибки, нам пришлось объявлять переменную:
var createMoveToCommand:Function;

Хотя есть случаи, в которых использование пространств имен себя оправдывает, но это не наш случай.

Iv
18.03.2008, 22:02
Я решил пойти по третьему пути: создать коллекцию методов и брать методы из нее.
Это быстрый вариант, понятный большинству разработчиков и в нем можно задействовать константы класса FormatSVG. Хотя без некоторых переделок не обойтись.

Опять протестируем на коротком примере.
Объявим статической константой коллекцию методов и добавим ссылку на метод:
private static const DRAW_METHODS:Object = new Object();
DRAW_METHODS[FormatSVG.MOVE_TO_ABSOLUTE] = createMoveToCommand;
Тестируем и обнаруживаем, что статическая константа не видит метод экземпляра класса. Ну, в общем это правильно, особенно учитывая то, что каждый метод привязан к экземпляру класса.

Мы можем создать такой экземпляр перед объявлением коллекции:
private static const INSTANCE:PathToArray = new PathToArray(null, null);
Тестируем, получаем ошибку. Причина ошибки понятна: мы передали невалидные аргументы. Чтобы от нее избавиться, достаточно в конструктор класса добавить проверку:
if (svgNode == null || dCmds == null) {
return;
}

Теперь добавляем тестовый вызов:
case FormatSVG.MOVE_TO_ABSOLUTE :
var drawMethod:Function = DRAW_METHODS[FormatSVG.MOVE_TO_ABSOLUTE];
trace("drawMethod: " +drawMethod);
j = drawMethod.call(this, svgCmds, j);
// j = createMoveToCommand(svgCmds, j);
break;

и опять обламываемся. Функция в коллекции найдена, но внутри функции всё равно идет обращение к экземплярам пустого объекта.
Чтобы избежать этого, передадим текущий объект в аргументах и внутри методов будем использовать обращение используя только его. А поскольку пустой экземпляр в таком случае не нужен, удаляем объявление константы INSTANCE и делаем метод createMoveToCommand статическим.
Вот что получается:

private static const DRAW_METHODS:Object = new Object();
DRAW_METHODS[FormatSVG.MOVE_TO_ABSOLUTE] = createMoveToCommand;

private function makeDrawCmds(svgCmds:Array):void {
....
case FormatSVG.MOVE_TO_ABSOLUTE :

var drawMethod:Function = DRAW_METHODS[FormatSVG.MOVE_TO_ABSOLUTE];
j = drawMethod(this, svgCmds, j);
// j = createMoveToCommand(svgCmds, j);
break;
....
}
private static function createMoveToCommand(instance:PathToArray, svgCmds:Array, j:int) : int {
// moveTo point
instance.firstP = instance.lastP = new Point(Number(svgCmds[j]), Number(svgCmds[j + 1]));
instance.drawingCommands.push(new FillCommand(instance.fill.color, instance.fill.alpha));
instance.drawingCommands.push(new StyleCommand(instance.stroke.width, instance.stroke.color, instance.stroke.alpha));
instance.drawingCommands.push(new MoveCommand(instance.firstP.clone()));
j += 2;
if (j < svgCmds.length && !isNaN(Number(svgCmds[j]))) {
do {
// if multiple points listed, add the rest as lineTo points
instance.lastP = new Point(Number(svgCmds[j]), Number(svgCmds[j + 1]));
instance.drawingCommands.push(new LineCommand(instance.lastP.clone()));
instance.firstP = instance.lastP;
j += 2;
} while (j < svgCmds.length && !isNaN(Number(svgCmds[j])));
}
return j;
}
Если мы аналогично заменим обращения ко всем методам, станет ли наш код от этого лучше? Нет, только не это.

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

Iv
19.03.2008, 17:19
Переименуем методы: makeDrawCmds в makeDrawCommands и extractCmds в extractCommands.
И перейдем к рефакторингу метода extractCommands.


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

private function pathDataToArray(node:XMLNode) : Array {
var dstring:String = "";
// if commas included, is it Adobe Illustrator (no spaces) or SVG Factory/other?
if (getAttribute(node, "d").indexOf(",") > -1) {
// has commas?
if (getAttribute(node, "d").indexOf(" ") > -1) {
// yes, has spaces?
// change spaces to commas, then deal as for Illustrator
dstring = String2.replace(getAttribute(node, "d"), " ", ",");
} else {
dstring = getAttribute(node, "d");
}
} else {
// no commas
// get rid of extra spaces and change rest to commas
dstring = getAttribute(node, "d");
dstring = String2.shrinkSequencesOf(dstring, " ");
dstring = String2.replace(getAttribute(node, "d"), " ", ",");
}
dstring = String2.replace(dstring, "c", ",c,");
dstring = String2.replace(dstring, "C", ",C,");
dstring = String2.replace(dstring, "S", ",S,");
dstring = String2.replace(dstring, "s", ",s,");
// separate the z from the last element
dstring = String2.replace(dstring, "z", ",z");
// change the following if M can be mid-path
dstring = String2.replace(dstring, "M", "M,");
dstring = String2.replace(dstring, "L", ",L,");
dstring = String2.replace(dstring, "l", ",l,");
dstring = String2.replace(dstring, "H", ",H,");
dstring = String2.replace(dstring, "h", ",h,");
dstring = String2.replace(dstring, "V", ",V,");
dstring = String2.replace(dstring, "v", ",v,");
dstring = String2.replace(dstring, "Q", ",Q,");
dstring = String2.replace(dstring, "q", ",q,");
dstring = String2.replace(dstring, "T", ",T,");
dstring = String2.replace(dstring, "t", ",t,");
// Adobe includes no delimiter before negative numbers
dstring = String2.replace(dstring, "-", ",-");
// get rid of any dup commas we might have introduced
dstring = String2.replace(dstring, ",,", ",");
// get rid of spaces
// (cr/lf's have to be removed before the xml object can be created,
// so that is done in xml.onData method)
dstring = String2.replace(dstring, " ", "");
dstring = String2.replace(dstring, "\t", "");

return dstring.split(",");
}

Перед началом скопированной части добавим вызов в методе extractCommands:
return pathDataToArray(node);
Тестируем, всё в порядке, удаляем ставший ненужным код в методе extractCommands.

Iv
21.03.2008, 14:53
Вернемся к методу extractCommands.
Он всё еще великоват по размеру и его логика теряется во множестве условных операторов. К тому же метод содержит логически обособленные блоки, которые проще воспринимать отдельными методами.
Для начала попробуем вынести блок if (hasFill).
В строке, следующей за if (hasFill) добавим:
fill = getFill();
C помощью CTRL+1 создадим метод и скопируем в него содержимое блока if до else. Закомментируем скопированный участок кода и из буфера обмена вставляем код в созданный метод.
Далее по очереди проходим по подсвеченым ошибкам и решаем что делать.
startColor - объявим локально.
node - объявим аргументом функции и передадим node в вызове.
thisColor - объявим локально.
Далее заменим присвоение переменной fill значения на return. Заодно удалим участки else, поскольку они оказываются ненужными. В итоге получаем такой метод:
private function getFill(node:XMLNode) : Fill {
// parse for fill color specification
// if a hex number is specified, startColor will be > 0
// if a color name is specified, startColor will be 0
var startColor:Number = getAttribute(node, "fill").indexOf("#") + 1;
if (startColor == 0) {
// name specified instead of color number
var thisColor:Number = colors[getAttribute(node, "fill")];
// if (thisColor == undefined) {
if (isNaN(thisColor)) {
return new Fill(0, 0); // set invisible if undefined
}
return new Fill(thisColor, 100);
}
return new Fill(parseInt(getAttribute(node, "fill").substr(startColor, 6), 16), 100);
}
Тестируем. Работает.

Сносим закомментированный код и этот участок кода превращается во вполне удобоваримый:
if (hasFill) {
fill = getFill(node);
} else {
fill = new Fill(0xffffff, 100);
}

Iv
21.03.2008, 15:01
На очереди следующий участок - блок if (hasStroke).
Поступаем аналогично предыдущему случаю и получаем такой метод:
private function getStroke(node:XMLNode) : Stroke {
// parse for stroke color specification
var startColor:Number = getAttribute(node, "stroke").indexOf("#") + 1;
if (startColor == 0) {
// name specified instead of color number
var thisColor:Number = colors[getAttribute(node, "stroke")];
// if (thisColor == undefined) {
if (isNaN(thisColor)) {
return new Stroke(0, 0, 0);
}
return new Stroke(colors[getAttribute(node, "stroke")], 0, 100);

}
return new Stroke(parseInt(getAttribute(node, "stroke").substr(startColor, 6), 16), 0, 100);
}
и вот такой вызов:
// stroke: color, width, alpha
if (hasStroke) {
stroke = getStroke(node);
} else { // if stroke is undefined, use invisible stroke
stroke = new Stroke(0, 0, 0);
}

Iv
21.03.2008, 15:28
Блок кода if (hasTransform) аналогичными действиями превращается в метод:
private function getRotation(node : XMLNode) : Number {
// parse for rotation specification
// hasRotate = getAttribute(node, "transform").indexOf("rotate");
// if (hasRotate > -1) {
var hasRotate:Boolean = getAttribute(node, "transform").indexOf("rotate") > -1;
if (hasRotate) {
var startRotate:Number = getAttribute(node, "transform").indexOf("(");
var endRotate:Number = getAttribute(node, "transform").indexOf(")");
return parseInt(getAttribute(node, "transform").substr(startRotate + 1, endRotate - startRotate));
}
return 0;
}

А вызов становится таким:
if (hasTransform) {
rotation = getRotation(node);
} else {
rotation = 0;
}

В этом месте я отметил для себя странность: переменная rotation локальна, мы ее присваиваем, но нигде не используем. Перенесем ее объявление ближе к присвоению и всё вместе закомментируем:
// var rotation:Number;
// if (hasTransform) {
// rotation = getRotation(node);
// } else {
// rotation = 0;
// }

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

Iv
21.03.2008, 16:51
Если вы помните, в самом начале мы определяли цели, к которым стремимся в процессе рефакторинга. Мы решили, что целью будет приведение кода в такой вид, при котором он мог бы стать основой для редактора SVG файлов.
Когда я смотрел статьи и спецификацию SVG формата, я обратил внимание на то, что на самом деле наш проект очень далек даже от базовых возможностей формата.
К примеру, если у нас будет SVG файл, содержащий окружность, заданную в соответствии со спецификацией, наш код не сможет ее отрисовать.
В нашу задачу не входит создавать недостающее, но мы должны сделать всё, для того, чтобы новые классы органично вошли в проект.
То что есть сейчас - отрисовка фигуры произвольной формы или пути. И мы знаем, что могут быть другие фигуры. В этой ситуации логично создать специальный пакет, в котором классы этих фигур смогут жить.
Второй немаловажный момент - необходимость перехода от представления пути к представлению редактируемой фигуры.
Тут понадобятся разъяснения: мы уже понимаем, что происходит: мы получаем строковые данные из атрибута d, разбиваем на конкретные команды в формате svg и преобразуем эти команды в удобоваримые для flash. К сожалению, в некоторых случаях это необратимая операция: кривые Безье третьего порядка, будучи разбитыми на последовательность кривых Безье второго порядка обратно уже не восстановить. А это требуется, поскольку для пользователя условия редактирования объекта не должны ухудшаться.

В итоге, систему нужно переводить на другие рельсы:
- данные формата должны сохраняться и конвертироваться только на этапе отрисовки;
- каждый тип XML узла SVG формата должен иметь соответствующий класс;

Iv
21.03.2008, 18:34
Помечаем в мозгу, что рефакторинг остановлен и мы начинаем заниматься разработкой.

Тактика, которую мы будем использовать такова: создаем классы и методы, которые считаем нужными, и с таким интерфейсом, который считаем удобным. Затем используем то, что нам необходимо из старого проекта, но не тупо переносим, а делаем это по аналогии, и с использованием возможностей AS3.

Начнем со структуры.
В папке svg создадим класс DocumentSVG.
В папке geom создадим папку lines и переместим в нее все классы папки geom, исправим ошибки, возникшие вследствие этого.
Вернемся к стандарту SVG, а именно к его части, описывающей остальные фигуры: http://www.w3.org/TR/SVG11/shapes.html#Introduction
и посмотрим, рисование каких фигур поддерживается:
path, rect, circle, ellipse, line, polyline, polygon.

Создадим папку shapes в папке geom. Создадим базовый класс для всех фигур: ShapeSVG.
И в этой же папке объявим интерфейс IShapeSVG:
package com.itechnica.svg.geom.shapes {
import flash.display.Graphics;

public interface IShapeSVG {

function update(svgNode : XML) : void;
function toSVG() : XML;
function toString() : String;
function draw(target : Graphics) : void;

}
}
В этой же папке создадим классы соответственно списку: PathShape, RectangleShape, CircleShape, EllipseShape, LineShape, PolyLineShape, PolygonShape. Все их унаследуем от
ShapeSVG и имплементируем интерфейс IShapeSVG.
По-началу все они будут выглядеть как близнеца братья, вот так:
package com.itechnica.svg.geom.shapes {
import flash.display.Graphics;

public class CircleShape extends ShapeSVG implements IShapeSVG {

public function update(svgNode : XML) : void {
}

public function toSVG() : XML {
return null;
}

public function toString() : String {
return "";
}

public function draw(target : Graphics) : void {
}
}
}

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

Wave
08.10.2008, 12:04
театр одного актера! :)

нескромный вопрос из зала:
как возможен рефакторинг без тестирования?

Iv
08.10.2008, 13:24
как возможен рефакторинг без тестирования?
- статью ты не читал видимо, поскольку в самом начале я просил сюда не писать, а делать это приватом.
- отсутствие формализованных тестов вовсе не означает, что тестирования нет (ты все-таки статью не читал).
- а во-вторых - можно. Представь себе. Особенно в данном случае, когда исходный AS2 код сам не проходит тестов. Мне что, приводить его в порядок и затем портировать?

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

И да, с ответами - в приват.