Форум Flasher.ru - Портируем, рефакторим, оптимизируем

Форум Flasher.ru (http://www.flasher.ru/forum/index.php)

- Статьи (http://www.flasher.ru/forum/forumdisplay.php?f=101)

- - Портируем, рефакторим, оптимизируем (http://www.flasher.ru/forum/showthread.php?t=109223)

Третий путь

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

Опять протестируем на коротком примере.
Объявим статической константой коллекцию методов и добавим ссылку на метод:

Код AS3:

private static const DRAW_METHODS:Object = new Object();

        DRAW_METHODS[FormatSVG.MOVE_TO_ABSOLUTE] = createMoveToCommand;

Тестируем и обнаруживаем, что статическая константа не видит метод экземпляра класса. Ну, в общем это правильно, особенно учитывая то, что каждый метод привязан к экземпляру класса.

Мы можем создать такой экземпляр перед объявлением коллекции:

Код AS3:

private static const INSTANCE:PathToArray = new PathToArray(null, null);

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

Код AS3:

if (svgNode == null || dCmds == null) {

        return;

}

Теперь добавляем тестовый вызов:

Код AS3:

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 статическим.
Вот что получается:

Код AS3:

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 лучше и достигли определенных успехов: теперь он читабелен. Но структура кода пока очень жесткая и мы столкнулись с тем, что не можем сделать этот участок лучше.
Стоит ли продолжать?
Нужно уметь остановиться и сказать себе, что время этой части кода еще не наступило.
Позднее, когда архитектура проекта станет лучше, мы вернемся к этому вопросу.

Рефакторинг метода extractCommands

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

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

Код AS3:

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:

Код AS3:

return pathDataToArray(node);

Тестируем, всё в порядке, удаляем ставший ненужным код в методе extractCommands.

Рефакторим extractCommands

Вернемся к методу extractCommands.
Он всё еще великоват по размеру и его логика теряется во множестве условных операторов. К тому же метод содержит логически обособленные блоки, которые проще воспринимать отдельными методами.
Для начала попробуем вынести блок if (hasFill).
В строке, следующей за if (hasFill) добавим:

Код AS3:

fill = getFill();

C помощью CTRL+1 создадим метод и скопируем в него содержимое блока if до else. Закомментируем скопированный участок кода и из буфера обмена вставляем код в созданный метод.
Далее по очереди проходим по подсвеченым ошибкам и решаем что делать.
startColor - объявим локально.
node - объявим аргументом функции и передадим node в вызове.
thisColor - объявим локально.
Далее заменим присвоение переменной fill значения на return. Заодно удалим участки else, поскольку они оказываются ненужными. В итоге получаем такой метод:

Код AS3:

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);

}

Тестируем. Работает.

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

Код AS3:

if (hasFill) {

        fill = getFill(node);

} else {

        fill = new Fill(0xffffff, 100);

}

На очереди следующий участок - блок if (hasStroke).
Поступаем аналогично предыдущему случаю и получаем такой метод:

Код AS3:

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);

}

и вот такой вызов:

Код AS3:

// stroke: color, width, alpha

if (hasStroke) {

        stroke = getStroke(node);

} else { // if stroke is undefined, use invisible stroke 

        stroke = new Stroke(0, 0, 0);

}

блок hasTransform

Блок кода if (hasTransform) аналогичными действиями превращается в метод:

Код AS3:

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;

}

А вызов становится таким:

Код AS3:

if (hasTransform) {

        rotation = getRotation(node);

} else {

        rotation = 0;

}

В этом месте я отметил для себя странность: переменная rotation локальна, мы ее присваиваем, но нигде не используем. Перенесем ее объявление ближе к присвоению и всё вместе закомментируем:

Код AS3:

//        var rotation:Number;

//        if (hasTransform) {

//                rotation = getRotation(node);

//        } else {

//                rotation = 0;

//        }

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

Необходимость структурных преобразований

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

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

К разработке

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

Тактика, которую мы будем использовать такова: создаем классы и методы, которые считаем нужными, и с таким интерфейсом, который считаем удобным. Затем используем то, что нам необходимо из старого проекта, но не тупо переносим, а делаем это по аналогии, и с использованием возможностей 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:

Код AS3:

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.
По-началу все они будут выглядеть как близнеца братья, вот так:

Код AS3:

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 (Сообщение 768740)

как возможен рефакторинг без тестирования?

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

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

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