patternjavascriptMinor
Auctioneer calling "Going once… going twice… sold!"
Viewed 0 times
oncetwicegoingsoldcallingauctioneer
Problem
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;
}
}
}
}Solution
Definetely not. The loop is useless (and un-optimizing) here.
As far as I can see, the index
Or, another valid alternative could be taking advantage of the resembling of the actions and generalize them by using data:
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.
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.
Code Snippets
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);var messages=[" zum ersten", " zum zweiten", " zum dritten", " verkauft!"];Context
StackExchange Code Review Q#111084, answer score: 5
Revisions (0)
No revisions yet.