1. JavaScript / Говнокод #15637

    +153

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    var cont_els = section.el.getElementsByClassName('cont');
for (var i = 0; i < cont_els.length; i++)
{
    var node_els = cont.el.getElementsByClassName('node');
    for (var i = 0; i < node_els.length; i++)

    Вложенный цикл переписывает i внешнего, и так бесконечно.

    Запостил: Itareo, 01 Апреля 2014

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

    • ava bormand 01.04.2014 21:34 # +2

      Жабаскрипт и его интуитивные скопы... Кто на это не налетал - тот никогда не писал на жс...
      Ответить

      • ava 1024-- 01.04.2014 21:49 # +3

        *стыдливо* я на это в сишке налетал, какой жс, Вы что
        Ответить

        • ava Itareo 01.04.2014 21:51 # +6

          это ж классика!
          Ответить

        • ava bormand 01.04.2014 21:59 # +2

          Ну в сишке все-таки переменная действует внутри блока, а не на всю функцию. Вот здесь в коде два var'а, а переменная i то одна!
          Ответить

          • ava Itareo 01.04.2014 22:00 # +2

            Ждем ECMASript 6!

            "В новой версии появится ключевое слово let, которое позволит объявлять переменные с блочной областью видимости"
            Ответить

          • ava 1024-- 01.04.2014 22:09 # +1

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

            • ava Itareo 01.04.2014 22:12 # +2

              Это жопа!

              var a = 5;
              function test() {
              console.log(a);
              var a = 123;
              }
              test(); // undefined!
              Ответить

              • ava 1024-- 01.04.2014 22:16 # +2

                На жс.ру был пример красивее, что-то вида
                var x = 10;
(function(){
  if (x < 2) {
    var x = x;
  }
  return x;
})()
                Ответить

                • ava Itareo 01.04.2014 22:18 # +1

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

            • ava bormand 01.04.2014 22:22 # +3

              > в котором переменные надо было определять строго в начале функции
              Да никогда вроде бы такого не было... Древние сишки требовали объявлять в начале блока.
              Ответить

              • ava 1024-- 01.04.2014 22:57 # +1

                Значит я ошибся. Вероятно, я тогда просто писал переменные в самое начало для надёжности.
                Ответить

          • ava Lokich 01.04.2014 22:39 # +4

            у меня было еще лучше. была вебслужба, которая возвращала объекты, которые могли содержать бесконечное количество вложенностей потомков. на клиенте же я рекурсивной функцией генерил html для него. ох какая же была боль, когда я написал вместо for(var i=0;i<object.Childs.length;i++) написал for(i=0;i<object.Childs.length;i++)... долго понять в чем причина :(
            Ответить

            • ava Itareo 01.04.2014 22:45 # +1

              еще одна классика)
              Ответить

            • ava bormand 01.04.2014 22:46 # +1

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

      • ava Itareo 01.04.2014 21:50 # +2

        В моем случае виновна привычка использовать функции underscore для перебора элементов. А как потребовалось в критичных местах увеличить скорость, так пришлось отказаться от библиотек. Отсюда и ошибки по непривычке))
        Ответить

        • ava wvxvw 01.04.2014 22:44 # +1

          Неужели прям такая разница?
          Если уж пытаться улучшить, то было бы логичнее использовать одну выборку типа '.cont > .node'.
          Еще не факт, что сформировать массивы из элементов, которые, судя по всему потом фильтровать будет выгоднее, чем пойти по пути nextSibling || firstChild.nextSibling. Но я сильно сомневаюсь, что такие уловки нужны, я еще ни разу не видел реальной необходимости оптимизации запросов к ДОМу (хотя накосячить можно везде, конечно).
          Ответить

          • ava bormand 01.04.2014 22:50 # 0

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

            • ava Itareo 01.04.2014 22:58 # +3

              Использование jQuery в десятки раз медленнее.

              Пруф: http://jsperf.com/jquery-class-vs-getelementsbyclassname-class/9

              Во всем проекте используется jQuery, но узкие места переписываю на чистом яваскрипте. Ускорение в десятки раз, без шуток.
              Ответить

          • ava Itareo 01.04.2014 22:59 # +2

            Разница большая, но на самом деле реальная необходимость в подобной оптимизации возникает редко, и в 99% случаев "из жизни" обычной скорости jQuery хватает.
            Ответить

            • ava wvxvw 01.04.2014 23:13 # +2

              Профайлер какой-то дурацкий, так понятно, что разница большая будет, потому что ж.квери нужно еще обертку создать. Но это все равно будет копеечной оптимизацией на фоне, например, грамотно построенного querySelector (в примере выше зачем-то делается два запроса к ДОМу, хотя можно было один). Ну и в зависимости от результатов и того, что нужно сделать с результатом (если в итоге нужно найти один-два элемента, или одну-две тысячи элементов, то нужна разная стратегия).

              ЗЫ. Когда-то, когда ДОМ АПИ были действительно очень медленными, был какой-то такой способ:
              function domWalker(callback, body) {
    var result = [];
    if (!body) body = document.body;
    if (callback(body)) result.push(body);
    if (body.firstChild)
        result.push.apply(result, domWalker(callback, body.firstChild));
    if (body.nextSibling)
        result.push.apply(result, domWalker(callback, body.nextSibling));
    return result;
}

              ХЗ. как оно соотносится с современными возможностями. Ну и понятно, что если сильно хочется производительности, то callback нужно вручную заинлайнить. Так написано для наглядности.
              Ответить

              • ava Itareo 01.04.2014 23:21 # +1

                Говорю по своему опыту - оптимизация действительно в десятки раз ускоряет. И в узких местах это очень помогает.

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

                • ava wvxvw 01.04.2014 23:34 # +1

                  Ну не может быть чтобы запрос к ДОМу имел критическое значение: на странице будет максимум тысяча-две элементов, т.е. в самом худшем случае, это один цикл на тысячу итераций: это ничего не стоит, как бы это не было сделано по сравнению с простым изменением размера какого-нибудь дива (т.как он бы вызвал перерисовку страницы, которая занимает 80% от времени выполнения типичного ж.скрипта).
                  Ответить

                  • ava Itareo 01.04.2014 23:42 # +1 

                    console.time('native');
for (var i = 0; i < 10000; i++) document.getElementsByClassName('comment-text');
console.timeEnd('native');
console.time('jQuery');
for (var i = 0; i < 10000; i++) $('.comment-text');
console.timeEnd('jQuery');


                    Прямо на этой странице.

                    Мои результаты после нескольких десятков запусков:
                    native - 16-39ms
                    jQuery - 286-312ms

                    Внутри jQuery регулярки для обработки селекторов, сам jQuery-обьект создается, + какие то кроссбраузерные проверки по идее могут быть.
                    Ответить

                    • ava wvxvw 01.04.2014 23:51 # +2

                      Ну и что? Ни одна вменяемая програма не требует 1000 запросов к дому. А даже если ее так написали, что она пытается столько запросов выполнить, ее гарантировано можно переписать с меньшим количеством запросов.
                      Не то оптимизируете / или не так. Оптимизировать запросы к ДОМу - имеет смысл если у вас там база данных на ХМЛ построена (т.е. там этого ДОМа десятки мегабайтов). Для обычной ХТМЛ страницы не может в принципе быть ситуации, когда запросы будут проблемой.
                      Ответить

                      • ava Itareo 02.04.2014 08:43 # +1

                        Естественно, 1000 запросов никому не нужно. Однако речь шла о том, ускоряет ли отказ от jQuery скорость, или нет.

                        И да, по поводу выборки ".cont > .node". такая штука не пойдет, потому, что мне нужно знать номер .cont, и еще сам .cont нужен, чтобы с ним кое что сделать.

                        "Для обычной ХТМЛ страницы не может в принципе быть ситуации, когда запросы будут проблемой."

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

                        Так же нужно уметь быстро сохранять все дерево элементов, поскольку требуется оперативное автосохранение.
                        Ответить

                        • ava wvxvw 02.04.2014 08:56 # +2

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

                          Код выше никак не поможет оперативно сохранить все дерево, и ж.квери тут тоже не помощник.
                          Ответить

                          • ava Itareo 02.04.2014 09:18 # +1

                            Судя по всему, я ничего не понимаю в оптимизации. Переписал код на джквери, получил ускорение в среднем на 10%. Откуда? Я же и так до этого нативные функции использовал, а джквери требуется еще селектор разобрать, + для каждой итерации он вызов функции генерирует, поскольку $().each() используется.
                            Ответить

                            • ava roman-kashitsyn 02.04.2014 09:26 # +2

                              @wvxvw говорит вам примерно о следующем.

                              Допустим, вы делаете линейный поиск в массиве.

                              Обнаружилось, что библиотечных indexOf проигрывает циклу на 20%. А если переписать узкое место сишке, получим ещё выигрыш на 70%. А если запараллелить поиск, то ещё на 40%.

                              Но тут выясняется, что ищем мы в ответе от базы занных, где правильный where даст гораздо больший performance win.
                              Ответить

                              • ava Itareo 02.04.2014 09:32 # +1

                                Ну значит, какая то внутренняя магия jQuery.

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

    • ava guest 18.04.2014 14:59 # 0

      Зачем это здесь? Просто глупая ошибка.
      Ответить

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