# Auctioneer calling “Going once… going twice… sold!”

I feel like the if-else combination isn't the most optimal and I'm also not sure if I need a loop

var Auktionator = function(){
var rennt = false;

this.versteigern = function(objekt){
if(rennt === false){
for(var i = 1; i<=4; i++){
if(i === 1){
setTimeout(function(){console.log(objekt + " zum ersten")},1000*i);
}
else if(i === 2){
setTimeout(function(){console.log(objekt + " zum zweiten")},1000*i);
}
else if(i === 3){
setTimeout(function(){console.log(objekt + " zum dritten")},1000*i);
}
else if(i === 4){
setTimeout(function(){console.log(objekt + " verkauft!")},1000*i);

}
rennt = true;
}
}
}
}

• I would put the words into a list and use i as the index – user193661 Nov 18 '15 at 8:58

## 2 Answers

Definetely not. The loop is useless (and un-optimizing) here.

As far as I can see, the index i is only used to calculate some constants' values. Realise that your original loop has the same effect than dropping off the loop and the ifs clauses, and hard-code the values computed from i:

setTimeout(function(){console.log(objekt + " zum ersten")},1000);
setTimeout(function(){console.log(objekt + " zum zweiten")},2000);
setTimeout(function(){console.log(objekt + " zum dritten")},3000);
setTimeout(function(){console.log(objekt + " verkauft!")},4000);


Or, another valid alternative could be taking advantage of the resembling of the actions and generalize them by using data:

var messages=[" zum ersten", " zum zweiten", " zum dritten", " verkauft!"];


This would be more comprehensive and flexible, since it allows to add more messages without changing the logic a little.

Update

I recommend RobH's answer, which is based upon arrays, and works fine.

• makes alot of sense! thank you very much! – Max Seifert Nov 18 '15 at 10:32
• Thanks, but I've realised that there is another better way. I've updated my answer. – Little Santi Nov 18 '15 at 14:50
• @RobH No, you are wrong. My loop is zero-based: (var i=0;i<messages.length;i++), so it never reaches i==4. – Little Santi Nov 18 '15 at 15:49
• @TopinFrassi - No. The i variable iscaptured, it's just that the loop executes to the end, setting i to 4 before the first setTimeout function is called. C# had the same behaviour for closures in a loop in .Net 4 but it was changed for .Net 4.5 – RobH Nov 18 '15 at 16:14
• @RobH You are right. :-( I deleted my update and recommended your answer. I can't delete my own post because it was selected as best. If the OP sets if off, I would delete it. – Little Santi Nov 25 '15 at 9:09

Little Santi's answer has a bug because it is closing over the i variable in that asynchronous call in the for loop. Here's a simplified version of that code running in Chrome's console:

The approach is a good one, you just need to copy the value of the i variable. There are many tricks for that but you can improve readability by introducing another function displayWithDelay.

var messages=[" zum ersten", " zum zweiten", " zum dritten", " verkauft!"];
var displayWithDelay = function (message, delay) {
window.setTimeout(function() { console.log(message); }, delay);
}

for (var i = 0; i < messages.length; i++)
{
displayWithDelay(messages[i], 1000*(i+1));
}