0

I pass in an array of error messages to parse. An example input would be:

"An item with this x Id already exists.
 An item with this y id already exists.
 An item with this barcode already exists.
"

That is, the string is literally each line above separated by a \n, with a final \n at the end.

function( msg )
{
  alert( "\"" + msg + "\"" );
  var aLines = msg.split( /\r?\n+/ );

  for ( var i in aLines )
  {
     if ( !aLines[i] ) { alert( "Error!" ); continue; }
     alert( i + ": \"" + aLines[i]  + "\"" );
  }
}

I split it into lines, and iterate over the lines. At index 3 there is no line and the first conditional triggers. Should that not be an empty line? e.g. ""

Then the loop actually goes one more element to 4, and shows a the contents of a function.

That is I get - five alerts:

0: "An item with this x Id already exists."
1: "An item with this y id already exists."
2: "An item with this barcode already exists."
Error!

The last one is most bizarre:

hasObject: "function(o) {
    var l = this.length + 1;
    ... more lines ...
}

I don't understand what is happening here. Why is it iterating over one more element? And why is the last element a function? And shouldn't offset 3 be an empty string? That is I should not be alerting "Error!" here.

Rafael Baptista
  • 11,181
  • 5
  • 39
  • 59
  • You should check the length of `aLines` and make sure it's not 4 because of a line-break at the end of the last line. – Kevin Boucher Oct 19 '12 at 19:11
  • `split` will return 4 segments in this case. The last segment will be an empty string (`""`), but not `null` or `undefined`. You need to ignore the last `\n` if it is on a line by itself. – Zabba Oct 19 '12 at 19:23
  • Problem solved. Every answer was correct. Thanks. – Rafael Baptista Oct 19 '12 at 19:27

4 Answers4

5

Never loop over an array with for...in.

Iterates over the enumerable properties of an object, in arbitrary order.

Why is using "for...in" with array iteration a bad idea?

Community
  • 1
  • 1
jbabey
  • 45,965
  • 12
  • 71
  • 94
  • 2
    This does not answer the question. – Zabba Oct 19 '12 at 19:22
  • @Zabba his issue is that he's getting array prototypes and other properties in the loop, which is caused by using the wrong looping construct. how does this not answer the question? – jbabey Oct 19 '12 at 19:34
1

As jbabey said, using for .. in loops in Javascript is risky and undeterministic (random order--sometimes). You would most commonly use that for parsing through objects in associative arrays. But, if you insist on keeping the for .. in, wrap the inside of the for with an if block like such:

for (var i in aLines)
{
    if(aLines.hasOwnProperty(i))
    {
        // ... Do stuff here
    }
}

Otherwise, just change it to a classic incremental for loop to get rid of that error:

for (var i = 0; i < aLines.length; i++)
{
   if ( !aLines[i] ) { alert( "Error!" ); continue; }
   alert( i + ": \"" + aLines[i]  + "\"" );
}
danyim
  • 1,274
  • 10
  • 27
1

You should use a regular for loop for arrays, because for-in will also return all the other property keys in the Array object. This is why you are seeing "hasObject" (and in my browser, I see a lot more after that): because your array has the function "hasObject", so when you enumerate all of Array's properties this comes up.

The correct for-loop:

  for ( var i = 0, ii = aLines.length; i<ii; i++ )
  {
    if ( !aLines[i] ) { alert( "Error!" ); continue; }
    alert( i + ": \"" + aLines[i]  + "\"" );
  }

Here is your code with the for-in loop replaced with a for loop, and it works as expected:

http://jsfiddle.net/SamFent/4rzTh/

Sam Fen
  • 5,074
  • 5
  • 30
  • 56
0

You are getting Error! in end because the splitting is giving you empty line "" and when you check for it with if(!aLines[i]) it returns true because it is empty/null/nothing. You can check it here in a fiddle, you remove the empty line from end and it does not go over array 4 times.

I have also put in following code which shows alert:

    var a="";
    if(!a){
        alert("!a");
    }
TheVillageIdiot
  • 40,053
  • 20
  • 133
  • 188