0

I have written a piece of code that for each entry of an array of users given, should call a function that modifies such user and sends email about their membership expiration.

The strange behaviour (I am quite sure I got how the scope works) are two:

  1. If I don't assign users[i] to foo, the line ldapclient.modify(users[i].dn, change, function(err) { does know what users[i] is, but the function does not ("error, cannot read property 'cn' of undefined")

  2. if I do assign foo, the function runs correctly - however, the result of sendMail is all the times the same (if I have 4 different users, there will be 4 sent emails which are the same and sent to one user)

Here's the code snippet

for ( var i = 0; i < users.length; ++i ){
    var foo = users[i];

    ldapclient.modify(foo.dn, change, function(err) {
       log.info({change: change, err: err}, 'Modifying Membership');
       assert.ifError(err);

       //send mail accordingly
       var mailmessage = {
         to:      foo.cn+" <"+foo.mail+">",
         subject: "Your membership has expired",
         text:    "Hi "+foo.cn+", \n your membership for the body " + foo.body + " \
                    has been suspended after automatic expiration"
       };
       mail.sendMail(mailmessage);
}

I was quite sure of having understood the scope and the magic of anonymous functions, async stuff, callbacks, and anything that makes node "cool", but this (mainly my point 2) just destroys all my beliefs.

linuxbandit
  • 2,362
  • 1
  • 14
  • 13
  • there is no "strange" behaviour, it's expected behaviour - foo, at the time the anonymous function is called (as a result of the asynchronous? `_ldapclient.modify`) will be `users[users.length -1]` – Jaromanda X Jan 20 '16 at 15:48
  • 1
    In your example `i` gets changed at the **end** of each loop and `foo` gets changed at the **start** of each loop. So when you use `i` you are using a value outside the array and when you use `foo` you are using the *last* value in the array. – Quentin Jan 20 '16 at 15:48
  • 1
    tl;dr: use `users.forEach` and avoid `for` loops. – zzzzBov Jan 20 '16 at 15:48
  • Sheeeet, I should improve my search skills here! I was sure I looked everywhere. Thanks for the fast reply – linuxbandit Jan 20 '16 at 15:52
  • Also, my choice of the `for` came after reading [this](http://stackoverflow.com/questions/9329446/for-each-over-an-array-in-javascript). It seemed the best thing but then the code evolved – linuxbandit Jan 20 '16 at 15:59

0 Answers0