Turning inefficient code into a loop? Javascript

Question

I have a webpage with colored boxes that look like this:

enter image description here

Each box has it's own div, the first one having an id of "one", second one being "two", and so on. It is supposed to be a guessing game where if you click the correct box you win, and if you pick the wrong box you lose. The correct box is randomly selected with a number generator. My javascript code is working, but I am wondering if there might be a more efficient way to write it? Here is what I have now:

function rand(){
  temp = Math.floor((Math.random()*6)+1);
  document.getElementById("one").onclick = one;
  document.getElementById("two").onclick = two;
  document.getElementById("three").onclick = three;
  document.getElementById("four").onclick = four;
  document.getElementById("five").onclick = five;
  document.getElementById("six").onclick = six; 
}

function correct(){
  alert("You are correct");
}

function incorrect(){
  alert("You are incorrect");
}

function one(){
 if (temp == 1){
   correct();
 }
 else {
   incorrect();
 }
}

function two(){
 if (temp == 2){
   correct();
 }
 else {
   incorrect();
 }
}

function three(){
 if (temp == 3){
   correct();
 }
 else {
   incorrect();
 }
}

The code goes on like this for all six boxes. I've been thinking for hours how I could do this with a loop or something, but I just can't figure it out. Any help would be much appreciated.

Here's a quick example of something that may work for you: jsfiddle.net/8neQ2/1 — Chase
– Chase, Commented Apr 25, 2013 at 2:24

Niet the Dark Absol · Accepted Answer · 2013-04-25 02:31:15Z

4

Since it's randomly generated, it really doesn't matter which box you click. To give an example, I have a minigame on my site that's a Scratch Card: there are twelve areas you can choose from, but you can only pick one. Internally it makes absolutely no difference which one you pick. The same applies here.

So, try this:

HTML:

<div id="board">
    <div id="one"></div>
    <div id="two"></div>
    ....
</div>

JS:

document.getElementById('board').onclick = function() {
    if( Math.random() < 1/6) {
        alert("You win!");
    }
    else {
        alert("Sorry, you lost...");
    }
}

EDIT: Here is a demonstration showing how to allow for more than one chance at winning. Note, however, that for any serious purpose you should submit a request to the server and have the server tell you if you won or lost, otherwise it's extremely easy to cheat ;)

edited Apr 25, 2013 at 2:31

answered Apr 25, 2013 at 2:14

Niet the Dark Absol

326k86 gold badges480 silver badges604 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Jace Over a year ago

This is way better than what I was shooting for!

Bryan Over a year ago

I'm thinking about giving the user more than one chance to guess correctly, and after they click on one that is incorrect, I want it to fade out so it isn't an option anymore. Not sure that code would work for that.

Niet the Dark Absol Over a year ago

It can be made to work for it. Let me put a Fiddle together for you.

Niet the Dark Absol Over a year ago

@user1157541 I have updated my answer with a demonstration :)

pocesar · Accepted Answer · 2013-04-25 02:30:52Z

1

using a DOM manipulation library like jQuery really helps in this case. But if you are willing to stay with vanilla javascript, you can use querySelectorAll/addEventListener event delegation, and check the id of the calling element, and work on that

check the working code on http://jsfiddle.net/KYvGf/

$(function(){
  var $box, $boxes = [], secret = Math.floor((Math.random()*6)+1);

  $(document).on('click', '.box', function(event){
    var $el = $(event.target);
    if ($el.data('id') === secret){
      alert('bingo!');
    }
  });

  for (var i = 1; i < 7; i++){
    $box = $('<div/>', {'class': 'box'});
    switch (i){
      case 1:
        $box.css({'backgroundColor': 'red'});
        break;
      case 2:
        $box.css({'backgroundColor': 'blue'});
        break;
      case 3:
        $box.css({'backgroundColor': 'green'});
        break;
      case 4:
        $box.css({'backgroundColor': 'yellow'});
        break;
      case 5:
        $box.css({'backgroundColor': 'purple'});
        break;
      case 6:
        $box.css({'backgroundColor': 'orange'});
        break;
    }
    $box.data('id', i);
    $boxes.push($box);
  }
  $('#holder').append($boxes);
});

<div id="holder"></div>

edited Apr 25, 2013 at 2:30

answered Apr 25, 2013 at 2:14

pocesar

7,0707 gold badges58 silver badges91 bronze badges

1 Comment

Bryan Over a year ago

I'm willing to use jQuery. Can you point me in the right direction? Don't have much experience with it.

Collectives™ on Stack Overflow

Turning inefficient code into a loop? Javascript

2 Answers 2

4 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related