Simplifing JavaScript Variables

Question

The following code works perfectly for what I am doing but I was wondering if there is a way to truncate it using an array.

function rollDice() {
   var d1 = Math.floor(Math.random() * 6) + 1;
   var d2 = Math.floor(Math.random() * 6) + 1;
   var d3 = Math.floor(Math.random() * 6) + 1;
   var d4 = Math.floor(Math.random() * 6) + 1;
   var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
   var strengthTotal = diceTotal;
   document.getElementById("strengthTotal").innerHTML = diceTotal;
}

I have tried putting all the variables into an array but can not for the life of me get the function to fire. Any assistance in being able to truncate this code would be greatly appreciated as there are going to be 7 different blocks all with 4 variables that will be fired at once. I can continue this function as is for the 7 different stats (i.e strengthTotal") however the block would be huge for something that should be very simple.

I dont know if i was really be clear enough with what I am trying to do. I am posting the full HTML, JavaScript, and CSS code so you all can see what I have so far. (when you see the full JScript you will probably either laugh, cry, cringe or all of the above) As i said I have a basic understanding from about 15 years ago and have not really worked with code like this for awhile. This way you can see what I am seeing. So here it goes.

HTML

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8" />
  <title></title>
  <link rel="stylesheet" href="dice.css">
  <script src="Script2.js"></script>
</head>
<body>
  <h2>Testing Differnet Types of Rolls</h2>
  <p> 4D6 Totaling 3 best rolls</p>   
  <button onclick="rollDice()">Roll Dice</button><br>
  <div>
    <p>Strength</p>
    <div id="strengthTotal" class="dice">0</div>
  </div>
  <div>
    <p>Dexterity</p>
    <div id="dexterityTotal" class="dice">0</div>
  </div>
  <div>
    <p>Constitution</p>
    <div id="constitutionTotal" class="dice">0</div>
  </div>
  <div>
    <p>Intelligence</p>
    <div id="intelligenceTotal" class="dice">0</div>
  </div>
  <div>
    <p>Wisdom</p>
    <div id="wisdomTotal" class="dice">0</div>
  </div>
  <div>
    <p>Charisma</p>
    <div id="charismaTotal" class="dice">0</div>
  </div>
</body>
</html>

JavaScript:

function rollDice() {
  var d1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var  3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
  document.getElementById("strengthTotal").innerHTML = diceTotal;
  var  1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var d3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
  document.getElementById("dexterityTotal").innerHTML = diceTotal;
  var d1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var d3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
  document.getElementById("constitutionTotal").innerHTML = diceTotal;
  var d1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var d3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
  document.getElementById("intelligenceTotal").innerHTML = diceTotal;
  var d1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var d3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
  document.getElementById("wisdomTotal").innerHTML = diceTotal;
  var d1 = Math.floor(Math.random() * 6) + 1;
  var d2 = Math.floor(Math.random() * 6) + 1;
  var d3 = Math.floor(Math.random() * 6) + 1;
  var d4 = Math.floor(Math.random() * 6) + 1;
  var diceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
document.getElementById("charismaTotal").innerHTML = diceTotal;
  }

and finally - dice.css:

div.dice {
  width: 32px;
  background: #f5f5f5;
  border: #999 1px solid; 
  padding: 10px;
  font-size: 24px; 
  text-align: center; 
  margin: 5px; 
}

Why not make one function that returns the value. You can then use that for all the different attributes. — Pointy
– Pointy, Commented Mar 14, 2016 at 20:30
Thank you for the prompt response. Is there any further detail that you could share about returning the value? — amfilipiak
– amfilipiak, Commented Mar 14, 2016 at 20:42
@amfilipiak I have an updated answer for your updated question. — Scott Marcus
– Scott Marcus, Commented Mar 14, 2016 at 22:47

Scott Marcus · Accepted Answer · 2016-03-14 22:53:01Z

2

Anytime you've got the same code more than once, you have an opportunity to refactor.

This will achieve what your code is looking for in a much more concise manner.

var attributes = null;

window.addEventListener("DOMContentLoaded", function(){
    document.getElementById("btnGo").addEventListener("click", rollDice);
    attributes = document.querySelectorAll(".dice");
});  

function rollDice() {
  
  // Loop over each characteristic DOM element to get a dice value for it
  for(var e = 0; e < attributes.length; ++e){
     
   var diceTotal = 0, d = [], roll = null;
    
   // roll the dice 4 times
   for(var i = 0; i < 4; ++i){
         roll = getRandom();  // Get the random die value
         diceTotal += roll;   // Add to previous total
         d.push(roll);        // Put current roll into array of roles
   }

   // After all rolls, subtract the lowest roll from the total
   // Math.min is great, but it expects individual values passed as 
   // arguments, not a single array. So, by calling the Math.min function
   // via the .apply() method, we supply the object to use for "this"
   // (Math) and then we supply an array to use for argument values.
   diceTotal -= Math.min.apply(Math, d);

   // These are only here to verify values: ************
   /*
   alert("Array contains: " + d);
   alert("Min value is: " + Math.min.apply(Math, d));
   alert("Dice total (minus min) is: " + diceTotal);
   */
   // ***************************************************

   // Place result in the current DOM element 
   attributes[e].innerHTML = diceTotal;
    
  }
}

function getRandom(){
   return Math.floor(Math.random() * 6) + 1;
}

div.dice {
  width: 32px;
  background: #f5f5f5;
  border: #999 1px solid; 
  padding: 10px;
  font-size: 24px; 
  text-align: center; 
  margin: 5px; 
}

<h2>Testing Differnet Types of Rolls</h2>
  <p> 4D6 Totaling 3 best rolls</p>   
  <button id="btnGo">Roll Dice</button><br>
  <div>
    <p>Strength</p>
    <div id="strengthTotal" class="dice">0</div>
  </div>
  <div>
    <p>Dexterity</p>
    <div id="dexterityTotal" class="dice">0</div>
  </div>
  <div>
    <p>Constitution</p>
    <div id="constitutionTotal" class="dice">0</div>
  </div>
  <div>
    <p>Intelligence</p>
    <div id="intelligenceTotal" class="dice">0</div>
  </div>
  <div>
    <p>Wisdom</p>
    <div id="wisdomTotal" class="dice">0</div>
  </div>
  <div>
    <p>Charisma</p>
    <div id="charismaTotal" class="dice">0</div>
  </div>

edited Mar 14, 2016 at 22:53

answered Mar 14, 2016 at 20:33

Scott Marcus

66k6 gold badges54 silver badges81 bronze badges

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

13 Comments

Edvaldo Silva de Almeida Jr Over a year ago

You just forgot to discard the smallest dice value, as in the code the OP posted.

amfilipiak Over a year ago

Thank you very much for your prompt response. I just attempted to use the above code and I am not getting any result. Once I click the button to call rollDice() it is not returning a value in the page. The html code for the button is below

amfilipiak Over a year ago

<button onclick="rollDice()">Roll Dice</button> is there anything else that needs to be done to call the second function?

Peter Over a year ago

Maybe make the number of dice an argument too.

IrkenInvader Over a year ago

This won't paste nicely but you aren't properly throwing away the low die -- we can store the rolls in an array and sort then remove the first element, then add up the remaining rolls.

var rolls = [];    for(var i = 0; i < 4; ++i){          rolls.push(getRandom());    }var diceTotal = rolls.sort().splice(1).reduce(function(a,b){ return a + b });

|

Observer · Accepted Answer · 2016-03-14 20:46:49Z

1

I am unable to leave comments due to my reputation.

You said: " Roll Dice is there anything else that needs to be done to call the second function? – amfilipiak 1 min ago "

<button></button> only works inside a form. I recommend giving the button an id, and then attaching an event listener.

document.getElementById('myButton').addEventListener('click', rollDice());

answered Mar 14, 2016 at 20:46

Observer

4632 silver badges8 bronze badges

3 Comments

Anatol - user3173842 Over a year ago

addEventListener('click', rollDice()) will attach the return value of rollDice, so unless it returns a function this won't work.

Observer Over a year ago

Thank you user3173842. addEventListener('click', function(){rollDice();}) would work though, right?

Anatol - user3173842 Over a year ago

Right, or just addEventListener('click', rollDice)

Edvaldo Silva de Almeida Jr · Accepted Answer · 2016-03-14 20:53:32Z

This is the correct code to sum and then discard the smallest dice value, according to the code posted.

function rollDice() {
  var rnd = 0;
  var minDice = 7;
  var diceTotal = 0;
  for(var i = 0; i < 4; ++i){
    rnd = getRandom(); 
    diceTotal += rnd;
    minDice = (rnd < minDice) ? rnd : minDice;
  }
  diceTotal -= minDice;

  // Not sure what your var strengthTotal = diceTotal was for
  document.getElementById("strengthTotal").innerHTML = diceTotal;
}

function getRandom(){
  return Math.floor(Math.random() * 6) + 1;
}

This code

document.getElementById("strengthTotal").innerHTML = diceTotal;

will only work provided you have something like

<div id="strengthTotal"></div>

or

<a id="strengthTotal"></a>

or yet

<span id="strengthTotal"></span>

i.e., provided you have a HTML container object to receive the value.

To execute the code you need something like:

<a href="javascript:void(0)" onclick="roolDice();">Roll dice</a>

Anatol - user3173842 · Accepted Answer · 2016-03-14 21:46:04Z

0

function rollDice(sides) {
  sides = sides || 6;
  return Math.floor(Math.random() * sides) + 1;
}

function rollDices(dices, sides) {
  var rolls=[], i;
  for (i = 0; i < dices; i++) {
    rolls.push(rollDice(sides));
  }
  rolls.sort().shift();
  return rolls.reduce(function(a, b) {
    return a + b;
  }, 0);
}

To run this on a button click you can do something like this:

window.addEventListener('DOMContentLoaded', function() {
  document.getElementById('roll').addEventListener('click', function() {
    var strengthTotal = rollDices(4);
    document.getElementById('strengthTotal').innerHTML = strengthTotal;
    // or
    var roll3d4 = rollDices(3, 4);
  });
});

<button id="roll">Roll</button>
<span id="strengthTotal"></span>

edited Mar 14, 2016 at 21:46

answered Mar 14, 2016 at 21:25

Anatol - user3173842

1,4251 gold badge13 silver badges18 bronze badges

Comments

Mitch Lamers · Accepted Answer · 2016-03-14 20:41:04Z

-2

function rollDice(aAmountOfDices) {
  var diceTotal = 0;
  for (i=0; i< aAmountOfDices; i++) {
      var d1 = getRandomNumber();
      var d2 = getRandomNumber();
      var d3 = getRandomNumber();
      var d4 = getRandomNumber();
      var thisDiceTotal = d1 + d2 + d3 + d4 - Math.min(d1, d2, d3, d4);
      diceTotal = diceTotal + thisDiceTotal;
  }
  return diceTotal;
}

function getRandomNumber() {
  return Math.floor(Math.random() * 6) + 1;
}

diceTotal = rollDice(6);
document.getElementById("strengthTotal").innerHTML = diceTotal;

This should do the trick

answered Mar 14, 2016 at 20:41

Mitch Lamers

1264 bronze badges

4 Comments

Pointy Over a year ago

Isn't your function rolling "amountOfDices" times 4?

Mitch Lamers Over a year ago

Oh woops I was under the impression there were 6 rounds of 4 dices being rolled. if this is not the case you could simply add randomNumber() to the diceTotal in the rollDice() method

Edvaldo Silva de Almeida Jr Over a year ago

You still have more variables than needed. Introducing an array was just to remove all these d1, d2... variables.

Mitch Lamers Over a year ago

Yes you are right, i just looked at Scott Marucs' answer and that is a better solution

Collectives™ on Stack Overflow

Simplifing JavaScript Variables

5 Answers 5

13 Comments

3 Comments

Comments

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

13 Comments

3 Comments

Comments

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related