Is it possible to remove a part of a function? (i.e. a specific variable when a condition is met)

Question

I'm trying to create a simple dice game using JavaScript. I want to simulate a Yahtzee game with 5 dice, and after each dice roll, every dice showing 6 should be put aside and the code should roll the remaining dice until all dice are put aside.

Here's what I've done so far:

<html>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<head>

<style>
div.dice{
float:left;
width:32px;
background:#F5F5F5;
border:#999 1px solid;
padding:10px;
font-size:24px;
text-align:center;
margin:5px;
}
</style>

<script>

function rollDice1(){
 var die1 = document.getElementById("die1");
 var d1 = Math.floor(Math.random() * 6) + 1;
 die1.innerHTML = d1;
}

function rollDice2(){
 var die2 = document.getElementById("die2");
 var d2 = Math.floor(Math.random() * 6) + 1;
 die2.innerHTML = d2;
}

function rollDice3(){
 var die3 = document.getElementById("die3");
 var d3 = Math.floor(Math.random() * 6) + 1;
 die3.innerHTML = d3;
 var status3 = d3;
}

function rollDice4(){
 var die4 = document.getElementById("die4");
 var d4 = Math.floor(Math.random() * 6) + 1;
 die4.innerHTML = d4;
}

function rollDice5(){
 var die5 = document.getElementById("die5");
 var d5 = Math.floor(Math.random() * 6) + 1;
 die5.innerHTML = d5;
}

function allDice() {
 rollDice1(); rollDice2(); rollDice3(); rollDice4(); rollDice5();
}

</script>

</head>

<body>

<div id="die1" class="dice">0</div>
<div id="die2" class="dice">0</div>
<div id="die3" class="dice">0</div>
<div id="die4" class="dice">0</div>
<div id="die5" class="dice">0</div>
<button onclick="allDice()">Roll Dice</button>

</body>

</html>

I've managed to create my 5 dice, and they also get assigned a new value with each roll. However, I've yet to manage to find a way to stop rolling the specific dice that show the value of six.

Ideally I would just be able to keep pressing the button labelled "Roll Dice" until all five dice show the value 6.

I've tried many methods, including putting all 5 roll function into an array and then removing the functions responsible for the specific dice that end up showing 6 after each roll. However, creating arrays seem to somehow affect my previous code. I've gotten error messages such as 'die1' is 'null' or that the function 'allDice' is undefined.

If needed I can add in some of the other versions of the program I've tried making work; I chose not to add them initially as this post felt way too long.

So, I've reached a point where I'm trying to create a function that rolls all 5 dice with each press of the button and I'm then looking for a way to remove each of these functions whenever their respective dice shows 6, or remove the variable that overrides the number shown on the dice from each of the rolling functions. If this is not possible or another solution is more suitable, then please write them below.

Ever head of arrays? You have six functions that do the same thing! — Savvas Parastatidis, Jun 09 '16 at 00:07

boehm_s · Answer 1 · 2017-07-13T18:54:51.847

3

You can turn your variables d1, d2, d3, d4, d5 to global variables and check their values before executing each rollDice function :

var d1, d2, d3, d4, d5;

function rollDice1(){
 var die1 = document.getElementById("die1");
 d1 = Math.floor(Math.random() * 6) + 1; // NO 'var'
 die1.innerHTML = d1;
}

...

function allDice() {
 if (d1 !== 6)
  rollDice1(); 

  ...

}

OR

var diceTab = [0, 0, 0, 0, 0, 0], i;
var dieTab = [die1, die2, die3, die4, die5]
function allDice() {
  for (i=0; i < diceTab.length; i++) {
    diceTab[i] =  Math.floor(Math.random() * 6) + 1;
    dieTab[i].innerHTML = diceTab[i];

    if (diceTab[i] === 6) {
      diceTab.splice(i, 1);
      dieTab.splice(i, 1);
    }
  }
}

ES6 very shorten version (UGLY) :

let dice = [1,2,3,4,5,6].map(r => document.getElementById(`die${r}`));
const [btn, roll] = [document.getElementById('btn'), function roll() {dice.forEach((die, i) => (result = die.innerHTML = Math.floor(Math.random() * 6) + 1) === 6 ? dice.splice(i, 1) : null)}];
btn.addEventListener('click', roll);

edited Jul 13 '17 at 18:54

answered Jun 08 '16 at 22:38

boehm_s

5,254
4
30
44

2

Still a lot of repetition in the code. Why not use an array to track die still in play? – Mister Epic Jun 08 '16 at 22:42
No, what I mean is you continue to write out very similar functions. This is a code smell and you need to DRY up your code. – Mister Epic Jun 08 '16 at 22:51
like that ? Splice to remove array cell when there is a 6 ? So we won't uselessly re-calculate a dice value and update the HTML . – boehm_s Jun 08 '16 at 23:06
That is definitely an improvement. +1 – Mister Epic Jun 08 '16 at 23:08

score 2 · Answer 2 · answered Jun 08 '16 at 23:01

2

Here is the most compact way I could think of (ES6 code):

let range = [1,2,3,4,5,6];
let dice = range.map(r => document.getElementById('die' + r))
const btn = document.getElementById('btn');

btn.addEventListener('click', roll);

function roll(){
    dice.forEach((die, index) => {
      result = Math.floor(Math.random() * 6) + 1;
      die.innerHTML = result;
      if(result === 6){
        dice.splice(index, 1);
    }
  })
}

https://jsfiddle.net/wbnhgr5a/

answered Jun 08 '16 at 23:01

Mister Epic

16,295
13
76
147

Could you explain what the second line of code does or point me towards somewhere I can read up on it? Otherwise really compact code, but I would not want to use it if I could not understand all of it. Thanks in advance! – Mathias Jun 08 '16 at 23:21
You can read about it here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map I should mention you probably don't want to use my code, as browsers wrong understand it as written. ES6 is the next generation of JavaScript, and what I wrote above needs to be transpiled into JavaScript of today before browsers can use it. It wouldn't be too hard to make it digestable for today's browsers though, let me know if you need a hand. – Mister Epic Jun 08 '16 at 23:36
This can sound like a 'troll', because it's unreadable, but here's a oneliner : `const roll = () => dice.forEach((die, i) => (result = die.innerHTML = Math.floor(Math.random() * 6) + 1) === 6 ? dice.splice(i, 1) : null);` – boehm_s Jul 13 '17 at 18:48

score 2 · Accepted Answer · 2016-06-08T23:48:44.843

If we look at your current die rolling algorithm we see that they are all the same except the element ID

function rollDice5() {
  var die5 = document.getElementById("die5");
  var d5 = Math.floor(Math.random() * 6) + 1;
  die5.innerHTML = d5;
}

When can dry up your code by passing in the one thing that is changing (the element id) as an argument

function rollDie(dieElementID){
    var die = document.getElementById(dieElementID);
    var newDieValue = Math.floor(Math.random() * 6) + 1;
    die.innerHTML = newDieValue;     
}

Element.innerHTML works for both setting and reading the element's HTML value. We can read what the current die is showing and determine if we need to roll it again.

function rollDie(dieElementID){
    var die = document.getElementById(dieElementID);
    if(die.innerHTML != 6){
        var newDieValue = Math.floor(Math.random() * 6) + 1;
        die.innerHTML = newDieValue;
    }
}

To roll all the dice we simply call our new generic rollDie and pass in the id.

function allDice() {
  rollDie("die1");
  rollDie("die2");
  rollDie("die3");
  rollDie("die4");
  rollDie("die5");
}

Every time we see a repetition of code that only has one thing changing we can try to dry it up. We can do this with allDice by storing a list of our die element ID's

function allDice() {
  var diceToRoll = ["die1","die2","die3","die4","die5"];
  for(var i = 0; i < diceToRoll.length; i++){
     rollDie(diceToRoll[i]);
  }
}

This is the minimum we need to get it to just work. We should also re-examine our rollDie function. Notice that we have introduced the side effect of not rolling when the die value is 6. We should break up rolling and checking if it is six to create flexibility in our code. To do this we can create two seperate functions and pass in the die element instead of the die id.

function rollDie(dieElement) {
  var newDieValue = Math.floor(Math.random() * 6) + 1;
  dieElement.innerHTML = newDieValue;   
}

function dieHasValue(dieElement, value) {
  return dieElement.innerHTML == value;
}

Then our allDice will look like this

function allDice() {
  var diceToRoll = ["die1", "die2", "die3", "die4", "die5"];
  for (var i = 0; i < diceToRoll.length; i++) {
    var dieElement = document.getElementById(diceToRoll[i]);
    if (!dieHasValue(dieElement, 6)) {
      rollDie(dieElement);
    }
  }
}

function rollDie(dieElement) {
  var newDieValue = Math.floor(Math.random() * 6) + 1;
  dieElement.innerHTML = newDieValue;
}

function dieHasValue(dieElement, value) {
  return dieElement.innerHTML == value;
}

function allDice() {
  var diceToRoll = ["die1", "die2", "die3", "die4", "die5"];
  for (var i = 0; i < diceToRoll.length; i++) {
    var dieElement = document.getElementById(diceToRoll[i]);
    if (!dieHasValue(dieElement, 6)) {
      rollDie(dieElement);
    }
  }
}

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

<html>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />

<head>
</head>

<body>
  <div id="die1" class="dice">0</div>
  <div id="die2" class="dice">0</div>
  <div id="die3" class="dice">0</div>
  <div id="die4" class="dice">0</div>
  <div id="die5" class="dice">0</div>
  <button onclick="allDice()">Roll Dice</button>

</body>

</html>

If you want to you can use the Array manipulation functions to make allDice look cleaner.

function getDie(elementId){
   return document.getElementById(elementId);
}
function dieNotShowingSix(dieElement){
   return !dieHasValue(dieElement,6);
}
function allDice() {
  ["die1", "die2", "die3", "die4", "die5"].
  map(getDie).
  filter(dieNotShowingSix).
  forEach(rollDie);
}

Nice and thorough explanation made this really easy to understand, thanks! — Mathias, Jun 09 '16 at 00:08

score 0 · Answer 4 · answered Jun 08 '16 at 22:43

Not that its a good solution, but to do this with minimal changes to your code just add a condition in each of the functions to check if the value is not equal to 6 :

function rollDice1(){ var die1 = document.getElementById("die1"); if(die1.innerHTML !== '6'){ var d1 = Math.floor(Math.random() * 6) + 1; die1.innerHTML = d1; } }

     function rollDice2(){
      var die2 = document.getElementById("die2");
      if(die2.innerHTML !== '6'){     
        var d2 = Math.floor(Math.random() * 6) + 1;
        die2.innerHTML = d2;
      }
     }

     function rollDice3(){
      var die3 = document.getElementById("die3");
      if(die3.innerHTML !== '6'){         
        var d3 = Math.floor(Math.random() * 6) + 1;
        die3.innerHTML = d3;
      }
      var status3 = d3;
     }

     function rollDice4(){
      var die4 = document.getElementById("die4");
      if(die4.innerHTML !== '6'){             
        var d4 = Math.floor(Math.random() * 6) + 1;
          die4.innerHTML = d4;
      }
     }

     function rollDice5(){
      var die5 = document.getElementById("die5");
      if(die5.innerHTML !== '6'){  
        var d5 = Math.floor(Math.random() * 6) + 1;
        die5.innerHTML = d5;
      }
     }

score 0 · Answer 5 · answered Jun 08 '16 at 22:44

function allDice() {

  var dices = [
    document.getElementById("die1"),
    document.getElementById("die2"),
    ...
  ]

  for each(var dice in dices) {
    if (dice.innerHTML != 6) //does not equal
    {
      dice.innerHTML=rollDice();
    } else {
      //put aside
    }
  }

}

tex · Answer 6 · 2016-06-08T23:05:46.173

There are many ways to do what you're trying to do in javascript, but I'm assuming you'd like an answer that you understand / that isn't too complicated, so I'll go with one that's a slight modification of your code, and I'll refactor it to make it shorter / less repetitive. Replace your javascript with the following:

var die1, die2, die3, die4, die5;
//Create one global variable per die

//We use one rollDice function that accepts the die number
function rollDice(whichDie){
  var die = document.getElementById("die"+whichDie);
  var dNum = Math.floor(Math.random() * 6) + 1;
  die.innerHTML = dNum;
}

//In allDice, we check each global die value and only roll the ones that 
//are not yet equal to 6
function allDice() {
  if (die1!==6) {rollDice(1)}
  if (die2!==6) {rollDice(2)}
  if (die3!==6) {rollDice(3)}
  if (die4!==6) {rollDice(4)}
  if (die5!==6) {rollDice(5)}
}

Or, for a sleeker, shorter version:

var diceCount = 5,
    diceEls;

while (diceCount--){
  diceEls[diceCount] = document.getElementById("die"+diceCount);
}

function allDice() {
  diceEls.forEach(function(el, index){
    var curVal = parseInt(el.innerHTML, 10);
    el.innerHTML = curVal===6 ? curVal : Math.floor(Math.random() * 6) + 1;
  })
}

Is it possible to remove a part of a function? (i.e. a specific variable when a condition is met)

6 Answers6