JavaScript. Problems with a loop

Question

Trying to create a piece of code that return all numbers below 1000 that are multiples of three. This is the relevant piece of code.

 <script>
 var i = 1;
 var mplesOf3 = [0];
 var myNum = 0;
 while (myNum < 1000){
 (3 * i) = myNum;
 mplesOf3.push(myNum);
 i++;
 };
 alert(mplesOf3);
 </script>

The code runs in a html page, hence the style tags and alert.

The code basically trying to do 3x1 then 3x2 then 3x3 so on and so forth until the result is over 1000. I came up with the concept days ago and I'm still not sure why its not running properly.

For the record I've seen other solutions to how to do this but because I'm learning and want to improve I want to know why this solution doesn't work.

Thank you in advance

Edit: I should have known the mistake would be something stupid. I wrote (3 x 1) = n on the pseudocode and just didn't spot the mistake because nothing seem wrong to me. Thank to all parties, will accept an answer when I can.

• Dave Newton: (3 * i) = myNum; Might want to re-check your JS basics and see how assignment works.
• user47589: (3 * i) = myNum; You have this assignment statement backwards. Almost every programming language, including JS, assigns the value of the right side to the left. You're attempting to assign the left side to the right, which is incorrect.
• epascarello: Look at your error console
• Amiga500: I think he thinks in a math world. In any case try reversing it. And also, the variable i is not used... your loop is infinite.
• Brian H.: @VedranMaricevic. the i is not used, but the loop is not infinite because it's checking on myNum

Answer 1

Your JavaScript engine should be telling you you have a syntax error (look in the web console if you're using a browser). You can't have an expression like (3 * i) on the left-hand side of an assignment. In JavaScript, the thing on the right of the = is evaluated, and assigned to the thing on the left.

Your algorithm would also result in 1002 being pushed, because you're not testing the result of setting myNum = 3 * i until after pushing.

Sticking with your original algorithm but fixing those two things:

var i = 1;
var mplesOf3 = [0];
var myNum;
while ((myNum = 3 * i) < 1000){
 mplesOf3.push(myNum);
 i++;
} // Note: No semicolon here, you don't put semicolons after blocks
 // attached to control-flow statements
console.log(mplesOf3);

This bit:

while ((myNum = 3 * i) < 1000){

evaluates 3 * i, assigns the result to myNum, and then checks that that value is < 1000.

That said, it would probably be simpler (fewer variables, less multiplication) to use a for and myNum += 3 in the increment section:

var mplesOf3 = [0];
var myNum;
for (var myNum = 3; myNum < 1000; myNum += 3) {
 mplesOf3.push(myNum);
}
console.log(mplesOf3);

There's also no particularly good reason to special-case the 0 like that, so I'd probably leave it out of the array initially and start counting from 0:

var mplesOf3 = [];
var myNum;
for (var myNum = 0; myNum < 1000; myNum += 3) {
 mplesOf3.push(myNum);
}
console.log(mplesOf3);

• infoage: Thank you. Thanks for going to the effort of making the simplest possible solution, and spotting the semi-colon error that no one else did.
• Cid: Upvoted. The semi-colon after the while scope isn't really an error. It can be interpreted as an empty instruction (Do nothing). It's just superfluous
• T.J. Crowder: @Cid - Right. I didn't say it was an error, I said it didn't go there. JavaScript does indeed have the empty statement. :-)
• Cid: @T.J.Crowder You indeed didn't, this was an answer to the first comment. And I guess every developper already did this mistake using an empty statement at the wrong place (while(condition); { ... }) :)
• T.J. Crowder: @Cid - Ouch. I got lucky and never did that, but again, ouch. :-) (I did, like most, flail about nonsensically with semicolons at first, though.)

Answer 2

This is invalid:

(3 * i) = myNum;

Instead, do this:

myNum = (3 * i);

I would do it this way, and this takes care that even the last one stays below 1000:

<script>
 var i, mplesOf3;
 mplesOf3 = [];
 for (i=0; i<1000; i++){
 if (i % 3 === 0){
 mplesOf3.push(i);
 }
 }
 alert(mplesOf3);
</script>

Update (see comments):

For better efficiency your code is better, and here is a complete fix that even takes care of the last value to be below 1000:

var i = 1;
var mplesOf3 = [0];
var myNum = 0;
while (myNum < 1000){
 myNum = 3 * i;
 myNum < 1000 && mplesOf3.push(myNum);
 i++;
};
alert(mplesOf3);

Another improvement would be to avoid the comparison in each loop and always remove the last item from the final array:

var i = 1;
var mplesOf3 = [0];
var myNum = 0;
while (myNum < 1000){
 myNum = 3 * i;
 mplesOf3.push(myNum);
 i++;
};
mplesOf3.pop();
alert(mplesOf3);

• infoage: Isn't this less efficient than the solution i'm trying to use, since your basically testing every between one and 1000 instead of just finding numbers that are multiple of three?
• Nabi: Yes, you are correct. However, it takes also care of the last item to be under 1000, and thus the answer marked as correct is actually wrong.

Answer 3

maybe your mistake is (3 * i) = myNum; you just need to: myNum=(3 * i); First name then assign a value;

• Brian H.: This will give one more result that expected, last value in the array will be 1002