Making a dice roller using the .addclass function not working

Question

I want id=die to load at first and then on clicking the button "click me" to load the specified class designed in CSS as 'die1' 'die2' and so on

function roll() {
  var die = Math.floor(Math.random() * 6);
  if (die == 1)
    $('#die').addClass('die1');
  else if (die == 2)
    $('#die').addClass('die2');
  else if (die == 3)
    $('#die').addClass('die3');
  else if (die == 4)
    $('#die').addClass('die4');
  else if (die == 5)
    $('#die').addClass('die5');
  else
    $('#die').addClass('die6');
}

#die {
  width: 300px;
  border: 5px solid black;
  padding: 25px;
  margin: 25px;
}

.die1 {
  width: 300px;
  border: 5px solid green;
  padding: 25px;
  margin: 25px;
}

.die2 {
  width: 300px;
  border: 5px solid pink;
  padding: 25px;
  margin: 25px;
}

.die3 {
  width: 300px;
  border: 5px solid violet;
  padding: 25px;
  margin: 25px;
}

.die4 {
  width: 300px;
  border: 5px solid yellow;
  padding: 25px;
  margin: 25px;
}

.die5 {
  width: 300px;
  border: 5px solid red;
  padding: 25px;
  margin: 25px;
}

.die6 {
  width: 300px;
  border: 5px solid blue;
  padding: 25px;
  margin: 25px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div id="die"></div>
<button onclick="roll()">Click me!</button>

I don't know why is it not calling the required class? When I tried printing the variable "die" in js it is working properly.

Isn't it because you don't use curly bracket btween your if statements like this: `if(die==1){ $("#die").addClass("die1"); } else if(die==2){ ... } ...` — Martijn Vissers, Sep 27 '18 at 09:24
@MartijnVissers nope , as long as its only one row , you can remove the brackets — Ashraf, Sep 27 '18 at 09:26
@Ashraf _“as long as its only one row , you can remove the brackets”_ - more correct way to phrase that would be, as long as it is only one _statement_. A “row” or line could contain multiple statements, but only the first one of those would be dependent on the if, everything else would not. — misorude, Sep 27 '18 at 09:36
Well, you learn something new everyday. Thanks for clearing that up — Martijn Vissers, Sep 27 '18 at 10:53

Zakaria Acharki · Accepted Answer · 2018-09-27T10:29:50.560

6

Your CSS rules should be more specific since the identifier is more specific than class, like :

#die.die1 {
#die.die2 {
...

Then you JS function could be simply like :

$('#die').removeAttr('class').addClass('die' + die);

Remove the previous class and add the new one.

NOTE: You may need to add the min value to the random as 1 since you don't have die0 class :

var die = Math.floor(Math.random() * 6) + 1;

function roll() {
  var die = Math.floor(Math.random() * 6) + 1;

  $('#die').removeAttr('class').addClass('die' + die);
}

#die {
  width: 300px;
  border: 5px solid black;
  padding: 25px;
  margin: 25px;
}

#die.die1 {
  width: 300px;
  border: 5px solid green;
  padding: 25px;
  margin: 25px;
}

#die.die2 {
  width: 300px;
  border: 5px solid pink;
  padding: 25px;
  margin: 25px;
}

#die.die3 {
  width: 300px;
  border: 5px solid violet;
  padding: 25px;
  margin: 25px;
}

#die.die4 {
  width: 300px;
  border: 5px solid yellow;
  padding: 25px;
  margin: 25px;
}

#die.die5 {
  width: 300px;
  border: 5px solid red;
  padding: 25px;
  margin: 25px;
}

#die.die6 {
  width: 300px;
  border: 5px solid blue;
  padding: 25px;
  margin: 25px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div id="die"></div>
<button onclick="roll()">Click me!</button>

If you want to avoid all those classes you could change the border-color from the JS code like :

function roll() {
  var colors = ["green", "pink", "violet", "yellow", "red", "blue"];
  var die = Math.floor(Math.random() * colors.length);

  $('#die').css('border-color', colors[die]).addClass('die' + die);
}

#die {
  width: 300px;
  border: 5px solid black;
  padding: 25px;
  margin: 25px;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div id="die"></div>
<button onclick="roll()">Click me!</button>

edited Sep 27 '18 at 10:29

answered Sep 27 '18 at 09:24

Zakaria Acharki

66,747
15
75
101

1

Math.floor(Math.random() * 6) + 1; would be correct to product 1,2,3,4,5,6 – Gerard Sep 27 '18 at 09:44
1

The +1 makes it good for the classes, but no more for the colors array. – Peter B Sep 27 '18 at 09:57
why you reopened the question? it was a perfect duplicate or because you replied to this one? – Temani Afif Sep 27 '18 at 10:04
That right @PeterB, updated to be `Math.floor(Math.random() * colors.length)` instead – Zakaria Acharki Sep 27 '18 at 10:30
@TemaniAfif I respect your point, but you would be in need to call something heavy than a regular answer when talking about specificity... – Zakaria Acharki Sep 27 '18 at 10:42
I can still add more duplicates if you don't agree and Specificity *is* the main issue here and the other answer pointed it and provided a link to explain it which is pretty enough ... I don't think you did more about the specifity issue. 90% of your answer is related to optimize and improve his code which is for me out of the scope of this site because it will only help the OP unlike the specificity issue that will help everyone ... Anyway, re-opening question like that without even commenting is a *rude* behavior for me when it's clear that the other contains the same answer. – Temani Afif Sep 27 '18 at 10:55
_I can still add more duplicates_ Then add them first to your suggested answer... – Zakaria Acharki Sep 27 '18 at 10:57

Making a dice roller using the .addclass function not working

1 Answers1