Problem
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.
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);
Solution
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
I enjoyed your question and the game you wrote, thanks! (:
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;
}
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 number18
). -
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 toparseInt()
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.