1. ActionScript / Говнокод #5296

    −152

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    /**
		 * 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;
		}

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

    Запостил: wvxvw, 16 Января 2011

    Комментарии (26) RSS

    • ava wvxvw 16.01.2011 17:18 # 0

      Да, это, до того, как это происходит:
      private var _people:Vector.<UserData> = new <UserData>[];
      Ответить

    • ava Lure Of Chaos 16.01.2011 18:21 # −1

      а вот здесь
      if ( !result )
      {
      result = _people[i];
      }

      i что будет держать?
      Ответить

      • ava babaika 16.01.2011 18:58 # 0

        _people.length
        k.o.
        Ответить

        • ava Lure Of Chaos 16.01.2011 19:06 # 0

          значит, _people[i], т.е. _people[_people.length] уже не имеет смысла
          Ответить

          • ava gegMOPO4 16.01.2011 19:24 # 0

            Ну так сказано же: If there is no such clientId, throws an error.
            Ответить

            • ava Lure Of Chaos 16.01.2011 19:34 # 0

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

              • ava gegMOPO4 16.01.2011 19:57 # +1

                Вот поэтому этот код и здесь. Шикарный ведь.

                P.S. И кто это вас постоянно минусует?
                Ответить

                • ava Lure Of Chaos 16.01.2011 20:28 # 0

                  > Вот поэтому этот код и здесь. Шикарный ведь.
                  по невнимательности (коммент проигнорил) даже не сразу оценил грандиозность замыслов автора.

                  > P.S. И кто это вас постоянно минусует?
                  Доброжелатели, очевидно.
                  Ответить

                  • ava gegMOPO4 16.01.2011 22:07 # +1

                    Если бы это была такая задумка, бросать исключение о выходе за пределы массива, следовало бы написать просто:
                    public function getUserDataByClientID(clientId : String) : UserData 
{
        for (var i:uint;; i++) 
                if (_people[i].userServerData.clientId == clientId) 
                        return _people[i];
}
                    Ответить

                    • ava wvxvw 16.01.2011 22:19 # 0

                      не скомпилируется (если указан возвращаемый тип, то все codepaths (во, не знаю как по-русски) должны возвращать значение).
                      А вообще, в итоге переделалось в 
                      return _people[clientId];
                      ну и 
                      private var _people:Object/*UserData*/ = {};
                      Завтра буду выяснять, а нужна ли функция вообще (а то может оказаться, что и весь класс снести можно...)
                      Ответить

                      • ava gegMOPO4 16.01.2011 22:26 # 0

                        Так за конец цикла выполнение никогда не попадает. Если компилятор отслеживает пути выполнения, то должен это понимать (и наоборот, ругаться, если бы за циклом что-то было).

                        Ну а переделка списка в ассоциативный массив -- это уже немножко другой уровень.

                        Автора кода поздравьте -- мы ждём ещё перлов от него.
                        Ответить

                        • ava Lure Of Chaos 16.01.2011 22:36 # −1

                          просто добавить return _people[i]; и все работает = )
                          Ответить

                          • ava wvxvw 16.01.2011 23:53 # 0

                            По правилам хорошего тона точка выхода из функции должна быть одна, и это должна быть последняя строчка в функции :) Куда уже там 2 раза return на две с половиной строчки? :) Я практически говорю, про конкретный компилятор (он по-сути в AS3 всего один - ASC), ну и его разновидности / надстройки MXMLC и т.п. Не скомпилирует. Он не на столько умный, чтобы понять, что такой цикл будет либо бесконечным, либо вернется что-то. Я не уверен, что тот же MSVC такое бы "одобрил"... javac тож вряд ли...
                            Ответить

                            • ava Lure Of Chaos 16.01.2011 23:58 # −2

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

                              вот так рождается говнокод... когда вместо немедленного завершения мы будем тянуть к единому выходу.
                              Ответить

                            • ava bugmenot 17.01.2011 00:50 # 0

                              > По правилам точка выхода из функции должна быть одна
                              пруф?
                              Ответить

                              • ava wvxvw 17.01.2011 01:09 # +1

                                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]
                                На самом деле кода не будет больше, и читать действительно удобнее. Я не всегда этого придерживаюсь, и не всегда это возможно, но в данном случает оно только к лучшему.
                                Ответить

                                • ava bugmenot 17.01.2011 04:31 # +2

                                  спасибо, доставило

                                  особенно кусок говна с надписью loading и градусник великолепен, прямо как в старых добрых девяностых

                                  хоть это правило и полная херня но фотошопных мальчиков надо держать в строгости
                                  Ответить

                                • ava gegMOPO4 17.01.2011 11:49 # +1

                                  Это рекомендация, а не требование. Тех, кто понимает преимущества и недостатки обоих подходов, она не касается. Тем более в такой простой функции.

                                  Бездумное следование этим рекомендациям и приводит к такому говнокоду.
                                  Ответить

                                  • ava wvxvw 17.01.2011 12:14 # 0

                                    Не правда, если делать бездумно, то лучше уже следовать каким-то рекомендациям, чем "интуиции". Потому что в последнем случае получаем идиотизм перемноженый на идиотизм (т.е. количество идиотизма в коде вырастет експотенциально).
                                    Ответить

                              • ava eth0 17.01.2011 20:52 # −1

                                Не знаю, как в приличных компиляторах, но, как мне кажется, при каждом return'е cbuilder (довольно старый) генерирует деструкторы для всех локальных переменных. Соответственно, ассемблерный код превращается в мешанину повторяющихся вызовов. Вопрос не исследован мной до конца.
                                Интересно будет посмотреть, что в таком случае делают другие компиляторы.
                                Ответить

    • ava gegMOPO4 16.01.2011 19:26 # 0

      Хороший, оригинальный говнокод.
      Ответить

    • ava wvxvw 16.01.2011 21:47 # 0

      Кроме всего вышесказаного, дальнейшее расследование показало, что clientId содержит то ли md5 то ли sha256, вобщем что-то, что призвано быть уникальным, что вообще сдeлало всю эту замечательную функцию бессмысленной :)
      Ответить

      • ava Lure Of Chaos 16.01.2011 21:54 # −2

        ну надо найти, она и ищет.
        а вообще сюда бы хороши карты(хеши)
        Ответить

    Добавить комментарий