CODEWITHSUNDEEP
javascriptregexloops
Snyte
Snyte

Reputation: 93

JavaScript - My function only triggers once while being called several times in a loop

I have this function :

  function simplifyString (string)
  {
    var charsToFind = new Array(/[áàâãä]/g, /[éèêë]/g, /[íìîï]/g, /[óòôõö]/g, /[úùûü]/g, /ç/g, /[-  \'&_]/g),
        charsToReplace = new Array('a', 'e', 'i', 'o', 'u', 'c', '');

    string = string.toLowerCase();

    for (i = 0; i < charsToFind.length; i++)
    {
      string = string.replace(charsToFind[i], charsToReplace[i]);
    }

    return string;
  }

And I use it inside a loop, like this :

for (i = 0; i < objects.length; i++)
{
  var value = simplifyString(objects[i].innerText);
  
  console.log(value);
  console.log(i);
 }

Objects variable contains an array of elements.

Console will only show me the inner text of the first element, and i will display the length of my array - 1.

If I remove the loop of my function, console will show me inner text of every object and the correct serie of numbers.

I don't quite understand this behaviour, any help?

Upvotes: 0

Views: 47

Answers (2)

cantuket
cantuket

Reputation: 1592

for (i = 0; i < objects.length; i++)
for (i = 0; i < charsToFind.length; i++)

TO...

for (var i = 0; i < objects.length; i++)
for (var i = 0; i < charsToFind.length; i++)

OR...

for (let i = 0; i < objects.length; i++)
for (let i = 0; i < charsToFind.length; i++)

Note on browser support: In IE 11 "let variables are not bound separately to each iteration of for loops" Caniuse - let.

You are currently declaring i, in both for loops, without an identifier var, let, const so it is being assigned to the same i property on the global object rather than declaring an new locally scoped variable. The for loop inside simplifyString (string) is assigning a new value to the same i as the first for loop and screwing everything up.


* As mentioned in comments, this how you could use reduce() to make your code more readable, minimize unnecessary side-effects/mutations and couple your tests directly with their replacement values to avoid some confusion...

var charReplacements = [
   { test: /[áàâãä]/g,   value: 'a' },
   { test: /[éèêë]/g,    value: 'e' },
   { test: /[íìîï]/g,    value: 'i' },
   { test: /[óòôõö]/g,   value: 'o' },
   { test: /[úùûü]/g,    value: 'u' },
   { test: /ç/g,         value: 'c' },
   { test: /[-  \'&_]/g, value: ''  }
];

function simplifyString (string) {    
    return charReplacements
              .reduce((str, {test, value}) => 
                 str.replace(test, value)
              , string);
}

NOTE: If you're running this over large datasets I'd ask someone smarter than me how do this with a few lines of regex, which would be much more efficient than iterating over the replacers

Upvotes: 1

FigletNewton
FigletNewton

Reputation: 160

The issue is with how your loop counters are scoped. Instead of doing

for (i = 0; ...)

You should declare your loop counter as

for (let i = 0;...)

So that it is scoped to your loop block.

If you declare your loop variable as just 'i=0', then 'i' has a global scope. In this scenario, the 'i' you are using in your simplifyString loop is the same 'i' that you are using in the loop that calls simplifyString! Since simplifyString increments 'i' after you call it the first time, the invoking loop terminates prematurely.

Upvotes: 1

Related Questions