Тестовое задание.

[AtomicFlasher]

Здравствуйте.
Ищу работу AS3 программиста. Сделал тестовое задание, но оно не было принято. Причина: не оптимизированный код. Прошу вас о небольшом code review. Что именно здесь не так?

TestTask.swf   (14.8 Кб)

[caseyryan]

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

var length:int = planets.length;
for (var i:int = 0; i < length; i++) {
   planets[i].update(target);
}

а не так:

for (var i:int = 0; i < planets.length; i++) planets[i].update(target);

Плюс у цикла должно быть тело, чтобы было удобнее читать. Ведь этот код не для себя пишется.

Второе. Известно, что доступ к константам всегда медленнее, чем доступ к локальным переменным.
Поэтому можно было часто используемые константы внутри функций "прилепить" к локальным переменным.
К примеру не использовать везде в функции вызов Math.PI
а сначала присвоить ее значение локальной переменной
var pi:Number = Math.PI;
а потом уже использовать ее.

По-поводу оператора switch. Здесь уже были темы с таким обсуждением, и приходили к выводу, что в ас3, if / else работает быстрее, чем switch. В отличие от некоторых других языков. Так что свич я бы тоже выкинул.

Так же есть такая мелочь как вид записи

if(moon)

неявное приведение к типу Boolean.
По логике, должна быть проверка типа, и после этого приведение к типу Boolean

if(moon != null)

А так должна сразу приводить к Boolean
Во многих языках первый тип записи вообще не позволит программе скомпилироваться.


Это так, то, что можно сказать на вскидку

[samana]

А на мой взгляд - работодатель не прав. В задании нет ни единого намёка на оптимизацию кода. А идеально оптимизированным кодом, для такого простого примера, вряд ли кто-то заморачивается, только конечно если это не очень опытный программер, у которого уже просто привычка оптимизировать всё что только можно.
Уверен, если бы в задании был намёк на оптимизацию, то код товарища AtomicFlasher был бы другим.

[caseyryan]

А на мой взгляд - работодатель не прав.

На мой тоже. Просто это был наверное вежливый способ отделаться от кандидата.

[max123456789]

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

Код достаточно хорош. А оптимизация - это отговорка...

P.S. Нужна срочная работа, выйдите на фриланс-сайты

[Котяра]

Если придираться, то я вижу здесь 3 недостатка:
1. Отрисовка графиксом, а не битмапами (хотелось бы видеть ваш фпс на 5000 объектов).
2. Отсутствие инвалидации. Т.е. отрисовка идёт каждый фрэйм независимо от сохранности данных.
3. Нет явного MVC, хотя по сути
Main - это контроллер, Planet - это модель, а Gui и Universe это вьюшки (рендереры)

Но код достаточно понятный и чистый.

[caseyryan]

В ТЗ ничего этого нет

[AtomicFlasher]

Сообщение от caseyryan

Просто видимо хотели проверить знания по разным нюансам.

Спасибо за развернутый ответ, но мне кажется, что все это экономия на спичках.

Сообщение от Котяра

1. Отрисовка графиксом, а не битмапами (хотелось бы видеть ваш фпс на 5000 объектов).
2. Отсутствие инвалидации. Т.е. отрисовка идёт каждый фрэйм независимо от сохранности данных.
3. Нет явного MVC, хотя по сути
Main - это контроллер, Planet - это модель, а Gui и Universe это вьюшки (рендереры)

1. Вы предлагаете при создании новых кругов отрисовывать их в BitmapData, а потом в EnterFrame рисовать их через copyPixel()?
2. Наверно не очень поможет - почти все двигается почти всегда. Но я подумаю об этом.
3. По-моему достаточно явное Как можно сделать его явнее в контексте такого маленького примера, чтобы это не выглядело как оверинжениринг?

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

[caseyryan]

Спасибо за развернутый ответ, но мне кажется, что все это экономия на спичках.

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

[Котяра]

Спасибо за развернутый ответ, но мне кажется, что все это экономия на спичках.

Дело в том, что код не обязательно должен следовать ТЗ. Особенно на собеседовании.
Сам принимал много тестовых заданий, причём со схожей тематикой.
И просто по коду видно, как человек относится к некоторым вещам. Кто-то явно прописывает MVC-роли в именах классов, кто-то начинает подключать всякие фрэймворки, кто-то оптимизирует отрисовку блиттингоми и использует всякие инвалидации. Не всегда получившийся оверхед таких решений хорош для решения конкретной задачи, но иногда очень неплох для определения кругозора тестируемого.
Одно из лучших решений просмотренных тестовых заданий на моём веку - это было решение от товарища ChuwY - он расписал по каждой задаче несколько вариантов решений, написал бенчмарктесты и описал в комментах лучшие и плохие стороны решения.

Добавлено через 37 минут

Как можно сделать его явнее в контексте такого маленького примера,

Просто выделить явно сущности - Main - пусть будет контроллер.
MainModel - содержит количество планет и лун
PlanetModel, содержит соответственно модель планеты и лун.
UniverseRenderer или MainView - рендерер вселенной
GUIView - гуи.
Когда имена сущностей более явные - сразу понятно, что некоторые вещи им нельзя позволять делать.