10
\$\begingroup\$

I did a simple dice game using what I learned about separating the 'view', 'model', and 'control'. I am quite clear on what the model is (it's the data and state). But I'm a bit confused on the difference between the View and Controller.

Here is a Codepen

This is the revised Codepen (following some suggestions)

var model = (function() {

  var diceSet = [];
  var diceToRemove = 0;
  var totalDiceRemoved = 0;
  var selectedDiceVal = 0;
  var permDiceLength = 0;

  return {
    diceSet: diceSet,
    diceToRemove: diceToRemove,
    totalDiceRemoved: totalDiceRemoved,
    selectedDiceVal: selectedDiceVal
  }

})();



var view = (function() {

  var domElements = {
    genDiceDOM: document.getElementById("trigger-gen-dice"),
    rollDiceDOM: document.getElementById("trigger-roll-dice"),
    diceContainerDOM: document.getElementById("dice-container"),
    messageDOM: document.getElementById("message"),
    genDiceContDOM: document.getElementById("container-init"),
    mainGameDOM: document.getElementById("container-main-game"),
    valueSelector: document.getElementById('choose-dice'),
    gameControllerDOM: document.getElementById('game-controller'),
    inputNumDice: document.getElementById("input-num-dice"),
    diceRollingDOM: document.getElementById("rolling-dice-anim")
  }

  var diceImages = {
    dice1: "https://image.ibb.co/mARW6J/dice_1.png",
    dice2: "https://image.ibb.co/ckY00d/dice_2.png",
    dice3: "https://image.ibb.co/gJ0DLd/dice_3.png",
    dice4: "https://image.ibb.co/drDpty/dice_4.png",
    dice5: "https://image.ibb.co/kqChDy/dice_5.png",
    dice6: "https://image.ibb.co/duJ00d/dice_6.png"
  }

  return {
    domElements: domElements,
    displayDiceSet: function(dataSet) {

      domElements.diceContainerDOM.innerHTML = "";

      dataSet.forEach(function(dice) {

        var diceDOM = document.createElement("div");
        var diceNumDOM = document.createElement("span");

        diceDOM.className = "dice";
        domElements.diceContainerDOM.appendChild(diceDOM);
        diceDOM.appendChild(diceNumDOM);
        // console.log("DICE: " + diceImages.dice1);
        // diceNumDOM.innerHTML = dice.value;
        diceNumDOM.innerHTML = `<img src="${diceImages["dice" + dice.value]}" width="50">`;

        if(dice.selected == true) {
          diceDOM.className += " selected";
        }

      });
    }
  }

})();



var controller = (function(viewCont, modelCont) {

  // Event Handlers
  viewCont.domElements.genDiceDOM.addEventListener("click", genNumDice);
  viewCont.domElements.rollDiceDOM.addEventListener("click", rollDice);
  viewCont.domElements.valueSelector.onclick = getSelectedVal;


  function Dice(name, value, selected, diceImage) {
    this.name = name;
    this.value = value;
    this.selected = selected;
    this.diceImage = diceImage;
  }


  function getSelectedVal(e) {
    var target = e.target;

    for (var i = 0; i < viewCont.domElements.valueSelector.children.length; i++) {
      // console.log("TEST");
      viewCont.domElements.valueSelector.children[i].className = "";    
    }

    target.className = "dice-selected";
    modelCont.selectedDiceVal = e.target.innerHTML;
  }


  function genNumDice() {

    viewCont.domElements.messageDOM.style.display = "block";

    if(viewCont.domElements.inputNumDice.value === "") {

      viewCont.domElements.messageDOM.innerHTML = `Please enter a number!`;  

    } else {

      var diceNumDOM =  parseInt((view.domElements.inputNumDice).value);

      viewCont.domElements.messageDOM.innerHTML = `Okay, I've generated ${diceNumDOM} pieces of dice!`;  

      for(i = 0; i < diceNumDOM; i++) {
        modelCont.diceSet[i] = new Dice("dice" + parseInt(i+1), null, false, null);
      }

      modelCont.permDiceLength = modelCont.diceSet.length;

      viewCont.domElements.genDiceContDOM.style.display = "none";
      viewCont.domElements.mainGameDOM.style.display = "block";

    }

  }


  function rollDice() {

    viewCont.domElements.messageDOM.style.display = "block";
    viewCont.domElements.diceContainerDOM.style.display = "none";

    viewCont.domElements.messageDOM.innerHTML = "Rolling the dice...";  
    viewCont.domElements.diceRollingDOM.style.display = "block";


    setTimeout(function(){ 

      viewCont.domElements.diceRollingDOM.style.display = "none";

      viewCont.domElements.diceContainerDOM.style.display = "block";
      viewCont.domElements.messageDOM.style.display = "block";

      modelCont.diceSet.splice(0, modelCont.diceToRemove);
      modelCont.diceToRemove = 0;

      var diceValueDOM = parseInt(modelCont.selectedDiceVal);

      modelCont.diceSet.forEach(function(dice, index) {
        var randomNum = Math.floor((Math.random() * 6) + 1);
        dice.value = randomNum;        
        // dice.diceImage = viewCont.diceImages.dice[randomNum].value;
        if(diceValueDOM === dice.value) {
          modelCont.diceToRemove++;
          modelCont.totalDiceRemoved++;
          dice.selected = true;
        } else {
          dice.selected = false;
        }
      });

      if(modelCont.diceToRemove > 0) {
        if(modelCont.totalDiceRemoved === modelCont.permDiceLength) { 
          viewCont.domElements.messageDOM.innerHTML = `I bet you drank a ton! No worries, you got it! Game has ended.`;
          viewCont.domElements.gameControllerDOM.style.display = "none";
        } else {
          viewCont.domElements.messageDOM.innerHTML = `Good guess! There's a "${ diceValueDOM }" in the set!`;
        }
      } else {
        viewCont.domElements.messageDOM.innerHTML = `There's no "${ diceValueDOM }" in the set. Drink, drink, drink!`;
      }

      viewCont.displayDiceSet(model.diceSet);

    }, 1000);

  }

})(view, model);
\$\endgroup\$
  • \$\begingroup\$ In javascript, it often doesn't make so much sense to separate the view and the controller, since they're so tightly connected (at least, when you are building a DOM app). That's why JS frameworks often employ MVW :Model - View - Whatever, or MVVM : Model - View - ViewModel. \$\endgroup\$ – Kokodoko May 15 '18 at 17:55
  • \$\begingroup\$ Here is, in Javascript, MVC stripped down to its essence \$\endgroup\$ – radarbob May 18 '18 at 23:05
8
+50
\$\begingroup\$

MVC thing is hard to be taken right. And yet it is the most popular (my guess) approach in the modern web development.

There are different ways to say what MVC means in particular application. I guess the cleanest part of the trio is the model. I'll call your model the DiceGame.

The DiceGame by itself is the most important part of your application. It stores valuable inhouse information and has methods to control the whole thing.

Let's see, how the DiceGame may look like and be used

var DiceGame = {
    dices: [],
    dicesInGame: 0,
    luckyDice: 0,
    drink: false,
    totalDrinks: 0,
    gameWon: false
};

DiceGame.start = function (num) {
    this.dicesInGame = num;
    this.totalDrinks = 0;
    this.gameWon = false;
};

DiceGame.roll = function (luckyDice) {
    this.luckyDice = luckyDice;
    this.dices = [];
    this.drink = true;
    var removedDices = 0;
    for (var i = 0; i < this.dicesInGame; i++) {
        var fairChoice = Math.floor(Math.random() * 6) + 1;
        if (fairChoice === luckyDice) {
            this.drink = false;
            removedDices++;
        }
        this.dices.push({
            value: fairChoice,
            lucky: fairChoice === luckyDice
        });
    }
    this.dicesInGame -= removedDices;
    if (this.drink) {
        this.totalDrinks++;
    }
    if (this.dicesInGame === 0) {
        this.gameWon = true;
    }
};

// Example
// press "Run code snippet" to see it in action

var count = 7; // Seven dices
var lucky = 6; // My lucky number for today

DiceGame.start(count);

while (!DiceGame.gameWon) {
    DiceGame.roll(lucky);
    console.log("dices to beat: " + DiceGame.dices.length);
    if (DiceGame.drink) {
        console.log("...drinking.");
    } else {
        console.log("lucky!");
    }
}

// I bet I am drunk already
console.log("Game won! Total drinks: " + DiceGame.totalDrinks);

This is how one may rewrite your model to be more self-centric.

Model knows nothing about how it will be used or shown. It has public and private parts and holds knowledge about how to shift its own state.

After every roll() call the properties luckyDice, dices and drink are allowed to be examined by view or controller.

In provided example all input was stored in two variables and console.log() calls were the only output. You may call this sort of a view. All other code from example was part of controller.

Finally, a short review of your code:

  • nicely formatted code
  • long variable names and lots of dots (property lookups) make your code less readable
  • all model logic resides in controller
  • controller tends to do lots of DOM manipulation - this is a job for a view.

I suggest you to move all displaying functions to the view and make your controller a short glue code.

Full example

var DiceGame = {
  dices: [],
  dicesInGame: 0,
  luckyDice: 0,
  drink: false,
  totalDrinks: 0,
  gameWon: false
};

DiceGame.start = function(num) {
  this.dicesInGame = num;
  this.totalDrinks = 0;
  this.gameWon = false;
};

DiceGame.roll = function(luckyDice) {
  this.luckyDice = luckyDice;
  this.dices = [];
  this.drink = true;
  var removedDices = 0;
  for (var i = 0; i < this.dicesInGame; i++) {
    var fairChoice = Math.floor(Math.random() * 6) + 1;
    if (fairChoice === luckyDice) {
      this.drink = false;
      removedDices++;
    }
    this.dices.push({
      value: fairChoice,
      lucky: fairChoice === luckyDice
    });
  }
  this.dicesInGame -= removedDices;
  if (this.drink) {
    this.totalDrinks++;
  }
  if (this.dicesInGame === 0) {
    this.gameWon = true;
  }
};



// View

var View = {
  // Public properties
  model: null,
  onSelectDiceNumber: function() {},
  onSelectDice: function() {}
};

View.start = function(el) {
  this.rootEl = el;
  this.messageEl = el.querySelector('.message');

  this.viewStartEl = el.querySelector('.view-start');
  this.diceNumberEl = this.viewStartEl.querySelector('.dice-number');
  this.selectDiceNumberEl = this.viewStartEl.querySelector('.select-dice-number');

  this.viewGameEl = el.querySelector('.view-game');
  this.rollResultsEl = this.viewGameEl.querySelector('.roll-results');
  this.diceEls = this.viewGameEl.querySelectorAll('.dice-guess');
  this.controlsEl = this.viewGameEl.querySelector('.controls');

  // Hide all views
  this.viewStartEl.style.display = 'none';
  this.viewGameEl.style.display = 'none';
  this.message(false);

  // Event listeners
  this.selectDiceNumberEl.addEventListener('click', (function(ev) {
    ev.preventDefault();
    this.onSelectDiceNumber(this.diceNumberEl.value);
  }).bind(this));

  for (var i = 0; i < this.diceEls.length; i++) {
    this.diceEls[i].addEventListener('click', (function(ev) {
      this.onSelectDice(+(ev.target.innerHTML));
    }).bind(this));
  }
};

View.showStart = function() {

  // Toggle views
  this.viewStartEl.style.display = '';
  this.viewGameEl.style.display = 'none';
};

View.showGame = function() {

  // Toggle views
  this.viewStartEl.style.display = 'none';
  this.viewGameEl.style.display = '';

  // Hide controls on game end
  this.controlsEl.style.display = this.model.gameWon ? 'none' : '';

  // Clear last roll results
  while (this.rollResultsEl.firstChild) {
    this.rollResultsEl.removeChild(this.rollResultsEl.firstChild);
  }

  // Add new roll results
  for (var i = 0; i < this.model.dices.length; i++) {
    var dice = this.model.dices[i];
    var el = document.createElement("div");
    el.innerHTML = dice.value;
    if (dice.lucky) {
      el.className = 'lucky';
    }
    this.rollResultsEl.appendChild(el);
  }
};

// Show status message
View.message = function(html) {
  if (html !== false) {
    this.messageEl.innerHTML = html;
  }
  this.messageEl.style.display = html === false ? 'none' : '';
};


// Controller

View.model = DiceGame;

View.onSelectDiceNumber = function(num) {
  num = parseInt(num, 10);
  if (num > 0) {
    View.message(false);
    DiceGame.start(num);
    View.showGame();
  } else {
    View.message("Select number");
  }
};

View.onSelectDice = function(num) {
  DiceGame.roll(num);
  if (DiceGame.gameWon) {
    View.message("You won!");
  } else {
    if (DiceGame.drink) {
      View.message("No dice, drink!");
    } else {
      View.message("Nice guess, " + DiceGame.dicesInGame + " left.");
    }
  }
  View.showGame();
}

View.start(document.getElementById("root"));
View.showStart();
#root {
  font-family: sans-serif;
  text-align: center;
  margin: 0 auto;
  max-width: 400px;
}

#root * {
  box-sizing: border-box;
  margin-bottom: 0.5em;
}

button,
input {
  width: 100%;
}

.roll-results>div {
  display: inline-block;
  background-color: #eee;
  width: 2em;
  height: 2em;
  line-height: 2em;
  margin-right: 1em;
}

.roll-results>div.lucky {
  font-weight: bold;
  border: 2px solid black;
}

.dice-guess {
  display: inline-block;
  width: 10%;
}

.message {
  background-color: #eee;
  color: #333;
  padding: 1em;
  margin-bottom: 1em;
}
<div id="root">

  <div class="message"></div>

  <div class="view-start">
    <div>Select number of dices:</div>
    <input class="dice-number" />

    <button class="select-dice-number">Start</button>
  </div>

  <div class="view-game">
    <div class="roll-results"></div>

    <div class="controls">
      <div>Select your lucky dice:</div>
      <div>
        <button class="dice-guess">1</button>
        <button class="dice-guess">2</button>
        <button class="dice-guess">3</button>
        <button class="dice-guess">4</button>
        <button class="dice-guess">5</button>
        <button class="dice-guess">6</button>
      </div>
    </div>
  </div>

I enjoyed your question and the game you wrote, thanks! (:

\$\endgroup\$
  • \$\begingroup\$ If I understand correctly, all the function in the controller must be in the model (rollDice, genNumDice, getSelectedVal)? And there must be separate functions in the view just for displaying these which the controller will help to communicate (e.g. controller says, "View, display the rolled dice from Model!") \$\endgroup\$ – catandmouse May 20 '18 at 0:57
  • \$\begingroup\$ E.g. rollDice is in Model while displayDiceRoll is in View? So Controller only tells displayDiceRoll to show whatever rollDice has generated? \$\endgroup\$ – catandmouse May 20 '18 at 1:40
  • \$\begingroup\$ @catandmouse, I've updated the answer and added example view and controller code. I hope it will give you some suggestions. \$\endgroup\$ – sineemore May 21 '18 at 8:58
  • \$\begingroup\$ But basicly yes. Controller pushes input from view to model. This way model knows nothing about view or controller. View knows public properties of model. Controller knows all public props and methods of both. \$\endgroup\$ – sineemore May 21 '18 at 9:14
  • \$\begingroup\$ Thanks! I did changes to my code above with revised codepen example. :) \$\endgroup\$ – catandmouse May 21 '18 at 21:58
4
\$\begingroup\$

MVC architecture

Obviously it can be difficult to break up the code in JavaScript to MVC components. Typically an instance of the model would hold information about each model item- in this case, a die. Perhaps a die model might be what you have created as the Dice function in the controller implementation. The values currently stored in Model could just be properties of the controller.

There is a multitude of articles about MVC in JavaScript online. I have not read many but I started looking at this Sitepoint article. It claims that the controller should let the view handle the DOM interactions (e.g. calling document.getElementById(), element.addEventListener(), etc. That way the controller has better separation (of concerns) from the view. The controller only has to worry about the logic of handling the model and delegating actions to the view.

I also looked at This Google Chrome Apps article about MVC architecture and it has a link to Addy Osmani's online book: Learning JavaScript Design Patterns. In the section about Views, the example shows a view that accepts a controller object and registers the event handlers using properties of the controller.

So its up to you how you structure it but I would remove DOM interactions from the controller and not include the DOM elements in the view’s returned object. It may be more appropriate for the view to have methods the controller can call, like to display a message, show various components, etc.

It appears your code uses the revealing module pattern and the view and model are passed to the controller. With this approach it may be tricky to subscribe the controller methods to events on the views DOM. One way might be to have a view method to register the callbacks. Or another way might be to use window.postMessage() to have the view communicate with the controller.

Other Feedback

  • I like how the DOM elements are cached in one object. I see many posts here where DOM elements are repeatedly looked up so this is a breath of fresh air!
  • I see this line within genNumDice():

    var diceNumDOM =  parseInt((view.domElements.inputNumDice).value);
    

    If you are going to use parseInt(), it is wise to specify the radix using the second parameter - unless you are using a unique number system like hexidecimal, octal, etc. then specify 10 for decimal numbers.

    Always specify this parameter to eliminate reader confusion and to guarantee predictable behavior. Different implementations produce different results when a radix is not specified, usually defaulting the value to 10.1

    var diceNumDOM =  parseInt((view.domElements.inputNumDice).value, 10);
    

    That way, if a value like 022 is entered, it isn't interpreted as the octal value (I.e. Decimal number 18).

  • Later in genNumDice(), I see this line:

    modelCont.diceSet[i] = new Dice("dice" + parseInt(i+1), null, false, null);
    

    i is an number (integer) so the call to parseInt() is superfluous

  • There is only one place where the dice are added to the DOM via appendChild(), and that should at most be called 6 times. If there were a lot more elements to be appended, it might be advantageous to use a DOMDocumentFragment or a container element that can be added to the DOM after all other elements are added to it in order to minimize reflows. For more tips on that, see this article. I know it is a few years old but has some good advice about DOM interactions.

\$\endgroup\$
  • 1
    \$\begingroup\$ +1 for mentioning DOMDocumentFragment! \$\endgroup\$ – sineemore May 18 '18 at 6:40
3
\$\begingroup\$

I'd appreciate any helpful review on my code in any area possible.


Here is, in Javascript, MVC stripped down to its essence


MVC parts are intended to be coherent, complete, objects necessarily differentiated by clear separation of concerns. This is the intent of the MVC pattern. For the model this means that it could be driven by a graphical or a console interface. However the OP code does not have these qualities. The model objects must have both state and functionality.

function Die (sides = 6) {
  this.thisRoll = null;
}

Die.prototype.toString = function() { return this.thisRoll; }

Die.prototype.roll = function(sides = 6) { 
   this.thisRoll = Math.floor((Math.random() * sides) + 1);
   return this.thisRoll; 
}

function Dice (howMany = 1 ){
  this.dice = [];
  this.thisRoll = [];

   for(var i=0; i<howMany; i++) { this.dice.push(new Die()); }
}

Dice.prototype.toString = function() { return this.thisRoll; }

Dice.prototype.roll = function {
      this.thisRoll.length = 0;  
      for(var die of this.dice) { this.thisRoll.push(die.roll()); }
      return this.thisRoll;
}

// not sure what to return. A count? Indexes of matching dice?
// perhaps some overloads to cover these scenarios
Dice.prototype.match = function(thisNumber) {
      return this.dice.reduce(
         (counter, die) => { 
            return die.thisRoll === thisNumber ? ++counter : counter;
         }
      );
   }

Now the view or controller only has to tell, don't ask the dice to roll. ("tell, don't ask" is confusing! I say "tell, don't do")


I see another model "class", let's call it GuessingDice. This is effectively the game itself. It will contain Dice, the player's guess, and a method, play() let's say, that makes everything happen.

The controller can access play() and wire it to the view - a button in a GUI or a keypress event in the console. MVC!

function GuessingDice () {
   this.dice;  // instantiated when player is prompted for how many;
   this.guess;

   var play = function(e) {
      /*
         The client - a GUI or console or ...? can call
         this guy because he got a reference to it via
         registerRollDiceHandler().
      */
   }
}

GuessingDice.prototype.initDice = function(howMany) { 
   this.dice = new Dice(howMany); 
}

GuessingDice.prototype.registerRollDiceHandler = function () {
   return play;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.