HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Auctioneer calling "Going once… going twice… sold!"

Submitted by: @import:stackexchange-codereview··
0
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 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.