Node.js for() loop returning the same values at each loop

Question

I'm making this really simple application to help me explore nodejs and I have a particular handler that generates HTML code based on the top10 messages in my database. The snippet I'm having problem with loops through the messages and call the function generating HTML and appends the result to my html string.

function CreateMessageboard(BoardMessages){
  var htmlMessageboardString = "";

  [... Console debug code ...]

  for(var i = 0; i < BoardMessages.length;i++){
        (function(){
            var j = i;
            console.log("Loading message %d".green, j);
            htmlMessageboardString += MessageToHTMLString(BoardMessages[j]);
          })();
  }
}

I think my problem is due to either Javascript's way of handling loops, related to closures from what I read and this is what I tried using above or the async way nodejs handles my function. Right now the 10 results are well returned from the db but the last message is processed at every loop.

I also tried, instead of doing var j = i, to take value i as function parameter and pass it in the closure and it returned the same results anyway.

I have a feeling I'm missing critical knowledge to solve my problem, can I get light on this matter ?

Edit : I'm welcome to give any other info on the code, I'd post the whole git repo but people probably don't want to swim through a whole project to help me debug this issue so I posted the whole function in the comments to provide more context.

It should work even without the function, since you don't have any other closures that could unintentionally capture i. Is that your exact code? — Matthew Flaschen
– Matthew Flaschen, Commented Apr 19, 2012 at 18:59
As @MatthewFlaschen says, the code looks fine. However, you can try function(i){......})(i); so that there's no doubt i is captured. — Pooria Azimi
– Pooria Azimi, Commented Apr 19, 2012 at 19:08
I suspect you need to include more context; the error doesn't appear to be here — Kato
– Kato, Commented Apr 19, 2012 at 19:08
Here is the complete code of the function and the console output gist.github.com/2423172, some MySQL output is from my mysql module. I highly doubt the error is somewhere else but I will keep on checking still. The bunch of undefined results are normal because I made a small error to post messages using user id 0 by default and I don't have such user, the last message is posted using this default user and is it's the one being repeated for each loop. — Tristan Dubé
– Tristan Dubé, Commented Apr 19, 2012 at 19:11
@Leroux, please have the code output the final HTML. It might also be helpful to put MessageToHTMLString(BoardMessages[messageNumber]) in a temporary variable and output that, before concatenating. After those changes, can you post the code and console output again? — Matthew Flaschen
– Matthew Flaschen, Commented Apr 19, 2012 at 22:25

Joe · Accepted Answer · 2012-04-19 19:45:40Z

26

  for(var i = 0; i < BoardMessages.length;i++){
        (function(j){
            console.log("Loading message %d".green, j);
            htmlMessageboardString += MessageToHTMLString(BoardMessages[j]);
        })(i);
  }

That should work; however, you should never create a function in a loop. Therefore,

  for(var i = 0; i < BoardMessages.length;i++){
        composeMessage(BoardMessages[i]);
  }

  function composeMessage(message){
      console.log("Loading message %d".green, message);
      htmlMessageboardString += MessageToHTMLString(message);
  }

answered Apr 19, 2012 at 19:45

Joe

82.8k18 gold badges130 silver badges147 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Matthew Flaschen Over a year ago

There's nothing wrong with creating an anonymous function in a loop, as long as the scoping is done correctly.

Joe Over a year ago

@MatthewFlaschen, Or take the complexity of correct scoping out of the equation.

ming_codes · Accepted Answer · 2012-04-19 19:36:31Z

3

I would suggest doing this in a more functional style :P

function CreateMessageboard(BoardMessages) {
  var htmlMessageboardString = BoardMessages
   .map(function(BoardMessage) {
     return MessageToHTMLString(BoardMessage);
   })
   .join('');
}

Try this

answered Apr 19, 2012 at 19:36

ming_codes

2,93227 silver badges24 bronze badges

Collectives™ on Stack Overflow

Node.js for() loop returning the same values at each loop

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related