Don't make functions within a loop (как убрать ошибку)?

Question

[Sergey750il]

Код работает нормально, но Sublime подсвечивает ошибку "Don't make functions within a loop". Как правильно вынести функцию из цикла для исправления ошибки?

for (var p = 0, len = number.length; p < len; p++) {
        if (number[p] == 'Свободен') {
            number_js[p] = $('.ofice:eq(' + p + ')>.ofice_number>strong').map(function() {
                return $(this).html();
            })[0];

            for (var i = 0, len = rsrGroups.length; i < len; i++) {
                if (number_js[p] == rsrGroups[i].data("room")) {
                    rsrGroups[i].attr('fill', '#F4DA6C');
                }
            }
        }
    }

Answer 1

[Алексей Уколов]

function getSomeHtml() {
        return $(this).html();
}

for (var p = 0, len = number.length; p < len; p++) {
        if (number[p] == 'Свободен') {
            number_js[p] = $('.ofice:eq(' + p + ')>.ofice_number>strong').map(getSomeHtml)[0];

            for (var i = 0, len = rsrGroups.length; i < len; i++) {
                if (number_js[p] == rsrGroups[i].data("room")) {
                    rsrGroups[i].attr('fill', '#F4DA6C');
                }
            }
        }
    }

Только функция тут вообще не нужна:

number_js[p] = $('.ofice:eq(' + p + ')>.ofice_number>strong').first().html();

Comments

[Дмитрий]

тоже хотел это предложить, но порой не хочется лепить в коде функции на 1 строку вместо анонимных.

[Алексей Уколов]

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

[Sergey750il]

Попрробовал самостоятельно убрать из цикла функцию к клику:

function sendAjax(){
$('.point').css('display', 'none');
$.ajax({
url: '/' + room_this + '-2/',
cache: false,
type: "POST",
beforeSend: function() {
$('.ddd').html('Please wait...');
},
success: function(html) {
$('.ddd').html(html);
}
});

}

Этот код вставил в тело цикла:
$('.btn').click(sendAjax);

Но проблема в том что, консоль выдает ошибку Uncaught ReferenceError: room_this is not defined и код не работает.

[Алексей Уколов]

Sergey750il: Если у вас "до" работало, а "после" - нет и вы хотите чтобы вам объяснили почему, нужно показывать варианты и до и после.

[Sergey750il]

Вот это вариант -до. С первым циклом понятно, а со вторым где клик не могу вынести функцию.

var rsrGroups = [path_ag, path_ah, path_aj, path_ak, path_al, path_am, path_ao, path_ap, path_aq, path_ar, path_ca, path_cb];

path_ag.data("room", "201").attr('cursor', 'pointer');
path_ah.data("room", "212").attr('cursor', 'pointer');
path_aj.data("room", "218").attr('cursor', 'pointer');
path_ak.data("room", "219").attr('cursor', 'pointer');
path_al.data("room", "202").attr('cursor', 'pointer');
path_am.data("room", "215").attr('cursor', 'pointer');
path_ao.data("room", "205").attr('cursor', 'pointer');
path_ap.data("room", "204").attr('cursor', 'pointer');
path_aq.data("room", "214").attr('cursor', 'pointer');
path_ar.data("room", "213").attr('cursor', 'pointer');
path_ca.data("room", "216").attr('cursor', 'pointer');
path_cb.data("room", "203").attr('cursor', 'pointer');

var number = $('.ofice .ofice_status>em').map(function() {
return $(this).html();
});

var number_js = [];

for (var p = 0, len = number.length; p < len; p++) {
if (number[p] == 'Свободен') {
number_js[p] = $('.ofice:eq(' + p + ')>.ofice_number>strong').map(function() {
return $(this).html();
})[0];

for (var i = 0, len = rsrGroups.length; i < len; i++) {
if (number_js[p] == rsrGroups[i].data("room")) {
rsrGroups[i].attr('fill', '#F4DA6C');
}
}
}
}
$('.ofice').prepend('Офис');
$('.ofice_number').after('.............................');
arr = new Array();
for (var i = 0, len = rsrGroups.length; i < len; i++) {

var el = rsrGroups[i];

el.mouseover(function() {
var point = this.getBBox(0);

$('#ofices').next('.point').remove();

$('#ofices').after($('').addClass('point'));

$('.point')

.css({
left: point.x + (point.width / 2) - -300,
top: point.y + (point.height / 2) - 850
})
.fadeIn();
$('.data_' + this.data('room') + '').clone()
.css('display', 'block').addClass('dopclass')

.appendTo(".point");
// $('.point .are-us,.point .price,.point hr').css('display', 'block');
$('.point .ofice_number').next().css('display', 'none');
$('.point .ofice_number').prev().css('color', '#6e594c');
var room_this = this.data('room');

$('.btn').click(function() {

$('.point').css('display', 'none');
$.ajax({
url: '/' + room_this + '-2/',
cache: false,
type: "POST",
beforeSend: function() {
$('.ddd').html('Please wait...');
},
success: function(html) {
$('.ddd').html(html);
}
});
});

});
$('#ofices').mouseout(function() {
setTimeout(function() {
$('.point').css('display', 'none');
return false;
}, 1000000);

});

}

[Sergey750il]

Там где по клику уходит Ajax запрос.

[Sergey750il]

Консоль пишет, что room_this не определен, но он же уже "родился" те я включаю функцию после рождения.

[Sergey750il]

Или ее нужно передавать как параметр к функции?

[Алексей Уколов]

Ну, как-то так - jsfiddle.net/koceg/z7btuwbd

[Алексей Уколов]

Вам надо понять области видимости в js, чтобы самостоятельно решать такие задачи. Это основа-основ.

[Алексей Уколов]

Вот так можно и обработчик клика в отдельную функцию вынести (решив проблему области видимости переменной) - jsfiddle.net/koceg/z7btuwbd/1

[Sergey750il]

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

[Sergey750il]

Алексей Уколов: Я читаю мануалы к JS, вроде все понятно, но на реальных задачах начинаю "спотыкаться".

[Sergey750il]

Не могли бы написать основные темы по JS к которым стоит уделить особое внимание?

[Алексей Уколов]

Sergey750il: основные темы, которым стоит уделить внимание

[Sergey750il]

Попробовал создать функцию вот так:

function sendAjax (room_this) {
$('.point').css('display', 'none');
$.ajax({
url: '/' + room_this + '-2/',
cache: false,
type: "POST",
beforeSend: function() {
$('.ddd').html('Please wait...');
},
success: function(html) {
$('.ddd').html(html);
}
});
return sendAjax;
}

И определить ее в теле цикла так:
$('.btn').click(sendAjax(this.data('room')));

Но почему то это не срабатывает. Хотя ошибок в консоле нет.

[Алексей Уколов]

Sergey750il: Попробуйте найти 10 отличий между вашим вариантом и моим.
Подсказка - 10 отличий вы не найдете, а вот одно очень существенное есть.

[Sergey750il]

У Вас все в функцию return обернуто, я попытался ввести return в конце тк мне это более привычно.

[Алексей Уколов]

Sergey750il: это же не вопрос привычки, это разное поведение. Попробуйте вникнуть в то, то происходит в коде, без этого вы программировать не сможете.

[Sergey750il]

Теперь вроде понял, что мы возвращаем весь код, а не какие то отдельные значения, поэтому весь код оборачивается в return.
А вообще на сколько необходимо выносить функции из циклов? На сайте Линтера написано очень убедительно на эту тему, но как обстоят дела на самом деле?

[Алексей Уколов]

Sergey750il: выносить функции ради вынесения функций, конечно, бессмысленный труд.
Причин делать это три:
1. Каждая функция - это объект, они едят память. 100 одинаковых функций съедят в 100 раз больше памяти, чем одна, это очевидно. Но в реальности, потребление не очень большое, поэтому можно не заморачиваться.
2. Если создавать, например, обработчик события и делать это через анонимную функцию в цикле, то этот обработчик потом нельзя будет удалить при необходимости - ссылки на анонимную функцию нигде нет. Если мы имеем одну функцию и передаем ее в качестве обработчика, то так же легко мы сможем ее потом снять. Но, прямо скажем, я в своей практике пользовался этой возможностью ровно 0 раз.
3. Самая главная причина - архитектурная. Лапшу из анонимных функций сложно читать, сложно поддерживать, невозможно разбить на независимые модули, невозможно переиспользовать в других местах.

[Sergey750il]

Алексей Уколов: Я тоже заметил, что когда убрал анонимные функции код стал понятнее. Но когда пишешь код то удобнее использовать анонимные функции тк не надо метаться по коду для исправления функции. И когда функция находится допустим в цикле проще понять как все устроено нежели каждый раз подыматься вверх для просмотра объявления функции. Хотя может мне просто так кажется тк я не привык к таким функциям.