Optimize Javascript function with many (maybe?) loops inside

Question

I have the following function using in my application:

applyFormSectionIdsToControls: function(formSections) {
 if (!formSections.length) return;
 var formSectionIds = [];
 for (var i = 0, ii = formSections.length; i < ii; i++) {
 formSectionIds.push(parseInt(formSections[i].id));
 };
 for (var i = 0, ii = formSections.length; i < ii; i++) {
 var affectedWidgets = $.grep(application_widgets, function(item) {
 return (($.inArray(item.widget_key, ['nextbutton', 'dropdown', 'radiobuttons']) >= 0) && (item.parent_id == formSections[i].id));
 });
 for (var j = 0, jj = affectedWidgets.length; j < jj; j++) {
 if (affectedWidgets[j].widget_key == 'nextbutton' && (typeof affectedWidgets[j].settings.default_next_step != 'undefined' && affectedWidgets[j].settings.default_next_step > 0)) {
 var _next_step = parseInt(affectedWidgets[j].settings.default_next_step);
 if ($.inArray(_next_step, formSectionIds) == -1) {
 affectedWidgets[j].settings.default_next_step = formSectionIds[_next_step - 1];
 }
 }
 if (affectedWidgets[j].widget_key == 'radiobuttons' || affectedWidgets[j].widget_key == 'dropdown') {
 for (var k = 0, kk = affectedWidgets[j].settings.answers; k < kk; k++) {
 var _next_step = parseInt(affectedWidgets[k].settings.answers[k].next_step);
 if ($.inArray(_next_step, formSectionIds) == -1) {
 affectedWidgets[k].settings.answers[k].next_step = formSectionIds[_next_step - 1];
 }
 }
 }
 }
 };
}

I'm looking for any hints how to refactor it, as I think there are too many for loops.

• Vogel612: That's just \$\mathcal{O}(n^2)\$. not quite sure what you mean by "too many for-loops ... sure there is some optimizations that I see, but ...
• 200_success: The title "Optimize Javascript function with many (maybe?) loops inside" applies to too many questions on this site to be useful. At the same time, you need to tell us what this code does, ideally with a sample of the corresponding HTML. (Press Ctrl-M to make a live demo.) See How to Ask.

Answer 1

Look, ma, no loops!

But seriously, if you were concerned about performance I don't think it's a problem of nested loops per se, $.grep in the loop concerns me much more. Especially since you're doing basically the same grep over and over!

After grep, neither item.parent_id nor i are used again. Looks like we can just cycle through all widgets in one $.grep, and that's exactly what we're going to do.

Also, what you're doing with item.widget_key begs for a switch statement - or, at least, having those radiobutton and dropdown typed twice bugs me and begs for a well-placed typo to cause lots of frustration.

I'm not sure I understand exactly what you do with default_next_step, so correctNextStep probably needs some refinement (like, what if next_step - 1 is not a valid forum section index either?) but the code is supposed to work exactly the same as yours. (or not work, hard to say without test cases:D)

const inArray = (needle, haystack) => (-1 != haystack.indexOf(needle));
const parseIntIfDefined = (item) => ('undefined' === typeof(item) ? null : parseInt(item));
const applyFormSectionIdsToControls = function(formSections) {
 if (!formSections.length) return;
 const formSectionIds = formSections.map((section) => parseInt(section.id));
 const correctNextStep = (nextStep) => {
 if (inArray(nextStep, formSectionIds)) return nextStep;
 return formSectionIds[nextStep - 1];
 };
 $.grep(application_widgets, (item) => {
 if (!inArray(parseInt(item.parent_id), formSectionIds)) return;
 switch (item.widget_key) {
 case 'nextbutton':
 const defaultNextStep = parseIntIfDefined(item.settings.default_next_step);
 if (defaultNextStep) {
 item.settings.default_next_step = correctNextStep(defaultNextStep);
 }
 break;
 case 'radiobuttons':
 case 'dropdown':
 item.settings.answers.map(
 (answer) => answer.next_step = correctNextStep(parseInt(answer.next_step))
 );
 break;
 }
 });
}