It's sends 4 times the amount of members

Question

So I want my bot in a certain server to send a dm to a random member every 10 minutes. And when my bot has sent everyone from the server a dm it sends a complete message.

But when i start the bot it sends 4 times the amount of members

if (message.content.startsWith(botconfig.prefix + 'dmall')) {
    console.log("demo");
    var list = message.guild.members.array();
    sendMessage(list);
  }
});


function sendMessage(list) {
  setTimeout(function () {
    for (i = 0; i < list.length; i++) {
      console.log(list.length);
    }

    console.log("I'm done, mate!");
    sendMessage(list);
  }, 10 * 1000);
}

CONSOLE:

demo
4 (is the amount of the members)
4
4
4
I'm done mate!

Why do you claim that the message is sent four times? Is it because the number `4` is logged four times (assuming `list` -> [4, 4, 4, 4]) ? — Umair Sarfraz, Dec 18 '19 at 17:34
Idk 4 is the amount members in my server idk why it sends 4 times the same — , Dec 18 '19 at 17:38
It's because you are running a `for` loop on `list.length` which is `4`. Does it make sense? — Umair Sarfraz, Dec 18 '19 at 17:39

score 1 · Accepted Answer · answered Dec 18 '19 at 18:17

This part of your code:

for (i = 0; i < list.length; i++) {
  console.log(list.length);
}

tells Javascript to run the statement:

  console.log(list.length);

list.length times. If list.length is 4 which it appears to be here, then you will see in the console

That's what the code was instructed to do.

I don't see any reason why you'd put that in a loop unless you want to output each array element separately. So, if you want to just output the length once, then replace this:

for (i = 0; i < list.length; i++) {
  console.log(list.length);
}

with this:

console.log(list.length);

In addition, if you were to use a for loop, you MUST declare all variables that you use. So, this:

for (i = 0; i < list.length; i++) {

is very dangerous. It relies on a higher scoped i which can easily conflict with other higher scoped i variables and create hard-to-figure out bugs. Every for loop should declare it's own loop variable as in:

for (let i = 0; i < list.length; i++) {

If you run your code in strict mode (which you should), the above for loop declaration would likely cause an error (which is a good thing because you'd immediately see the coding error and fix it).

score 0 · Answer 2 · answered Dec 18 '19 at 17:38

There are a couple of things wrong here. For of: JavaScript variable scoping messing you up (Different example here

Second of all: setTimeout is not the same as setInterval.

Completely fixing your issue would be:

 // Use the discord API
let users = ['justme']
function getUsers() {
    // Mutate just to display a new one each time
    users = [...users, users.length.toString()];
    return users
}

function notifyUsers() {
    // Retrieve all users now
    const currentUsers = getUsers()
    // Send a message to each user
    currentUsers.forEach(user => sendMessage(user))
    console.log('I have sent everyone a message')
}

function sendMessage(user) {
  console.log(`Sending message to ${user}`)
}

// Start the application by looping every <whatever>
setInterval(() => notifyUsers(), 1000)

It's sends 4 times the amount of members

2 Answers2