Нашли или выдавили из себя код, который нельзя назвать нормальным,
на который без улыбки не взглянешь?
Не торопитесь его удалять или рефакторить, — запостите его на
говнокод.ру, посмеёмся вместе!
/**
* Returns UserData object of the user with specific clientId from the friend list.
* If there is no such clientId, throws an error.
* @return <B>com.gixoo.videoRound.data.users.UserData</B>
*/
public function getUserDataByClientID(clientId : String) : UserData
{
var result : UserData;
for (var i:uint; i < _people.length; i++)
{
if (_people[i].userServerData.clientId == clientId)
{
result = _people[i];
break;
}
}
if ( !result )
{
result = _people[i];
}
return result;
}
Тестирование показало, что ошибку эту никто не ловит... но зачем так сложно ее выбрасывать...
меня замыкает ситуация, когда пишут код не для того, что бы он работал, а что бы он не работал.
Хорошо, что это экшонскрипт со всеми проверками, а написать бы это на Си, и получаем не эксепшон, а трудноотлавливаемый баг.
Если бы это была такая задумка, бросать исключение о выходе за пределы массива, следовало бы написать просто:
public function getUserDataByClientID(clientId : String) : UserData
{
for (var i:uint;; i++)
if (_people[i].userServerData.clientId == clientId)
return _people[i];
}
не скомпилируется (если указан возвращаемый тип, то все codepaths (во, не знаю как по-русски) должны возвращать значение).
А вообще, в итоге переделалось в
return _people[clientId];
ну и
private var _people:Object/*UserData*/ = {};
Завтра буду выяснять, а нужна ли функция вообще (а то может оказаться, что и весь класс снести можно...)
Так за конец цикла выполнение никогда не попадает. Если компилятор отслеживает пути выполнения, то должен это понимать (и наоборот, ругаться, если бы за циклом что-то было).
Ну а переделка списка в ассоциативный массив -- это уже немножко другой уровень.
Автора кода поздравьте -- мы ждём ещё перлов от него.
По правилам хорошего тона точка выхода из функции должна быть одна, и это должна быть последняя строчка в функции 🙂 Куда уже там 2 раза return на две с половиной строчки? 🙂 Я практически говорю, про конкретный компилятор (он по-сути в AS3 всего один - ASC), ну и его разновидности / надстройки MXMLC и т.п. Не скомпилирует. Он не на столько умный, чтобы понять, что такой цикл будет либо бесконечным, либо вернется что-то. Я не уверен, что тот же MSVC такое бы "одобрил"... javac тож вряд ли...
http://opensource.adobe.com/svn/opensource/flexpmd/bin/flex-pmd-ruleset-creator.html
Открыть категорию Maintainability Rules, в ней самое первое правило, ну или если оно будет не первым, то вот его содержание:
[quote]A method should have only one exit point, and that should be the last statement in the method[/quote]
На самом деле кода не будет больше, и читать действительно удобнее. Я не всегда этого придерживаюсь, и не всегда это возможно, но в данном случает оно только к лучшему.
Не правда, если делать бездумно, то лучше уже следовать каким-то рекомендациям, чем "интуиции". Потому что в последнем случае получаем идиотизм перемноженый на идиотизм (т.е. количество идиотизма в коде вырастет експотенциально).
Не знаю, как в приличных компиляторах, но, как мне кажется, при каждом return'е cbuilder (довольно старый) генерирует деструкторы для всех локальных переменных. Соответственно, ассемблерный код превращается в мешанину повторяющихся вызовов. Вопрос не исследован мной до конца.
Интересно будет посмотреть, что в таком случае делают другие компиляторы.
Кроме всего вышесказаного, дальнейшее расследование показало, что clientId содержит то ли md5 то ли sha256, вобщем что-то, что призвано быть уникальным, что вообще сдeлало всю эту замечательную функцию бессмысленной 🙂
if ( !result )
{
result = _people[i];
}
i что будет держать?
k.o.
Хорошо, что это экшонскрипт со всеми проверками, а написать бы это на Си, и получаем не эксепшон, а трудноотлавливаемый баг.
P.S. И кто это вас постоянно минусует?
по невнимательности (коммент проигнорил) даже не сразу оценил грандиозность замыслов автора.
> P.S. И кто это вас постоянно минусует?
Доброжелатели, очевидно.
А вообще, в итоге переделалось в ну и Завтра буду выяснять, а нужна ли функция вообще (а то может оказаться, что и весь класс снести можно...)
Ну а переделка списка в ассоциативный массив -- это уже немножко другой уровень.
Автора кода поздравьте -- мы ждём ещё перлов от него.
вот так рождается говнокод... когда вместо немедленного завершения мы будем тянуть к единому выходу.
пруф?
Открыть категорию Maintainability Rules, в ней самое первое правило, ну или если оно будет не первым, то вот его содержание:
[quote]A method should have only one exit point, and that should be the last statement in the method[/quote]
На самом деле кода не будет больше, и читать действительно удобнее. Я не всегда этого придерживаюсь, и не всегда это возможно, но в данном случает оно только к лучшему.
особенно кусок говна с надписью loading и градусник великолепен, прямо как в старых добрых девяностых
хоть это правило и полная херня но фотошопных мальчиков надо держать в строгости
Бездумное следование этим рекомендациям и приводит к такому говнокоду.
Интересно будет посмотреть, что в таком случае делают другие компиляторы.
а вообще сюда бы хороши карты(хеши)