patternjavascriptMinor
Card-fighting game Part 2
Viewed 0 times
gamefightingcardpart
Problem
This is part 2 of the game I am building. After some good feedback on my first post I decided it was time to post my updated code.
The differences in this part are: Updated code after feedback, new dodge mechanic and better formatted code.
What I am looking for are: performance feedback, code style feedback, things I can do in a better way and JavaScript best practices.
For more information about the game I should advice you to visit my first question.
```
var cards = [1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 7, 7, 8, 8, 8, 8, 9, 9, 9, 9, 10, 10, 10, 10];
var shuffledCards = shuffle(cards);
var playerDeck = shuffledCards.slice(0, 20);
var computerDeck = shuffledCards.slice(20);
var playerlife = 25;
var computerLife = 25;
var playerFeedback = document.getElementById('playerFeedback');
var playerName = '';
var swordSound = new Audio('sounds/sword.mp3');
var blockSound = new Audio('sounds/block.mp3');
var dodgeSound = new Audio('sounds/dodged.mp3');
var dodged = false;
/**
* @param array
@returns {}
*/
function shuffle(array) {
"use strict";
var m = array.length;
var t;
var i;
// While there remain elements to shuffle…
while (m) {
// Pick a remaining element…
i = Math.floor(Math.random() * m--);
// And swap it with the current element.
t = array[m];
array[m] = array[i];
array[i] = t;
}
return array;
}
/**
* @returns {boolean}
*/
function getPlayerName() {
"use strict";
playerName = document.getElementById('userInput').value;
if (playerName.length === 0) {
return false;
}
document.getElementById('playerName').innerHTML = playerName;
document.getElementById('overlay').style.display = 'none';
}
function checkIsGameOver() {
"use strict";
var gameResult = document.getElementById('gameResult');
var gameResultContent = gameResult.innerHTML;
var replay = document.getElementById('replay');
var
The differences in this part are: Updated code after feedback, new dodge mechanic and better formatted code.
What I am looking for are: performance feedback, code style feedback, things I can do in a better way and JavaScript best practices.
For more information about the game I should advice you to visit my first question.
```
var cards = [1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 7, 7, 8, 8, 8, 8, 9, 9, 9, 9, 10, 10, 10, 10];
var shuffledCards = shuffle(cards);
var playerDeck = shuffledCards.slice(0, 20);
var computerDeck = shuffledCards.slice(20);
var playerlife = 25;
var computerLife = 25;
var playerFeedback = document.getElementById('playerFeedback');
var playerName = '';
var swordSound = new Audio('sounds/sword.mp3');
var blockSound = new Audio('sounds/block.mp3');
var dodgeSound = new Audio('sounds/dodged.mp3');
var dodged = false;
/**
* @param array
@returns {}
*/
function shuffle(array) {
"use strict";
var m = array.length;
var t;
var i;
// While there remain elements to shuffle…
while (m) {
// Pick a remaining element…
i = Math.floor(Math.random() * m--);
// And swap it with the current element.
t = array[m];
array[m] = array[i];
array[i] = t;
}
return array;
}
/**
* @returns {boolean}
*/
function getPlayerName() {
"use strict";
playerName = document.getElementById('userInput').value;
if (playerName.length === 0) {
return false;
}
document.getElementById('playerName').innerHTML = playerName;
document.getElementById('overlay').style.display = 'none';
}
function checkIsGameOver() {
"use strict";
var gameResult = document.getElementById('gameResult');
var gameResultContent = gameResult.innerHTML;
var replay = document.getElementById('replay');
var
Solution
Organize the data
Sound Library
You're starting to get a lot of global variables in your project. As your project grows, you'll want to organize it more so that it is easier to grasp. Some of your data would be better organized into classes and objects. For example you could make a
Now when you want to add another sound or edit the ones you have, all that logic will be contained in the
Player Class
You might want to consider making a player class. There are a few ways to make classes in JavaScript, but I prefer a factory method (a function that constructs an instance of your class from parameters):
Deck Class
You have a lot of global functions that you're applying to a deck of cards. And a lot of your functions need to know that your decks are arrays. This deck class abstracts your deck logic into one place:
Now you can refactor your
This might not seem like much, but now your decks are able to manage themselves. They've become their own module. Your
DOM Element Library - Calling
Nearly every single one of your functions needs to access a DOM element at some point. You could introduce jquery to simplify the redundancy of
I really like these two functions that I found in a cheeky little article about the "harmfulness" of jquery:
```
// Returns first element that matches CSS selector {expr}.
// Querying can optionally be restricted to {container}’s descendants
function $(expr, container) {
return typeof expr === "string"? (container || document).querySelector(expr) : expr || null;
}
// Returns all elements that match CSS selector {expr} as an array.
// Querying can optionally be restricted to {container}’s descendants
fun
Sound Library
You're starting to get a lot of global variables in your project. As your project grows, you'll want to organize it more so that it is easier to grasp. Some of your data would be better organized into classes and objects. For example you could make a
sounds library object for sound effects:var sounds = {
block: new Audio('sounds/block.mp3'),
dodge: new Audio('sounds/dodged.mp3'),
sword: new Audio('sounds/sword.mp3')
};
sounds.sword.play(); // usageNow when you want to add another sound or edit the ones you have, all that logic will be contained in the
sounds object.Player Class
You might want to consider making a player class. There are a few ways to make classes in JavaScript, but I prefer a factory method (a function that constructs an instance of your class from parameters):
function createPlayer(name, maxLife) { // factory functions
var player = {
name: name,
life: maxLife,
deck: undefined // no cards yet
};
return player;
}
var player = createPlayer(undefined, 25); // usage
var computer = createPlayer("Hunter", 25);Deck Class
You have a lot of global functions that you're applying to a deck of cards. And a lot of your functions need to know that your decks are arrays. This deck class abstracts your deck logic into one place:
function createDeck(array) {
var cards = array; // private variable
return {
// removes the card off the top of the deck
draw: function () {
return cards.pop(); // the backside of the array the top of the deck
},
// returns the value of the card on the top of the deck
peek: function () {
return cards[cards.length - 1];
},
// returns the size of the deck
size: function () {
return cards.length;
},
// randomizes the order of the cards in the deck
shuffle: function () {
cards = shuffle(cards); // call your global shuffle function
},
// returns an array to be assigned to two players. Second array get's the extra card if the deck's length is odd
deal: function () {
var half = Math.floor(cards.length / 2);
var deck0 = createDeck(cards.slice(0, half));
var deck1 = createDeck(cards.slice(half));
return [deck0, deck1];
},
// returns a string of the deck
toString: function () {
return cards.toString();
}
};
}
var deck = createDeck([1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8];
deck.shuffle();
var hands = deck.deal();
player.deck = hands[0];
computer.deck = hands[1];Now you can refactor your
whoWon function like so:function whoWon() {
"use strict";
var playerCard = player.deck.draw();
var computerCard = computer.deck.draw();
document.getElementById('computerDamageImage').setAttribute('src', 'images/empty.png');
document.getElementById('playerDamageImage').setAttribute('src', 'images/empty.png');
if (playerCard > computerCard) {
computerLife = computerLife - (playerTopCard - computerTopCard);
sounds.sword.play();
document.getElementById('computerDamageImage').setAttribute('src', 'images/damage.gif');
playerFeedback.innerHTML = computer.name + ' lost: ' + (playerTopCard - computerTopCard) + ' HP';
} else if (playerCard < computerCard) {
playerlife = playerlife - (computerTopCard - playerTopCard);
sounds.sword().play();
document.getElementById('playerDamageImage').setAttribute('src', 'images/damage.gif');
playerFeedback.innerHTML = playerName + ' lost: ' + (computerTopCard - playerTopCard) + ' HP';
} else {
sounds.block.play();
playerFeedback.innerHTML = 'It was a tie!';
}
checkIsGameOver();
}This might not seem like much, but now your decks are able to manage themselves. They've become their own module. Your
whoWon function doesn't need to know that the cards are stored as an array; It doesn't need to know that the top of the deck is the front of the array or the back. All it knows is that the decks have a draw method that "draws" a card from a players deck.DOM Element Library - Calling
document.getElementById like 100000 timesNearly every single one of your functions needs to access a DOM element at some point. You could introduce jquery to simplify the redundancy of
document.getElementById, but then you would have to include jquery, adding a dependency to your code that you don't really need.I really like these two functions that I found in a cheeky little article about the "harmfulness" of jquery:
```
// Returns first element that matches CSS selector {expr}.
// Querying can optionally be restricted to {container}’s descendants
function $(expr, container) {
return typeof expr === "string"? (container || document).querySelector(expr) : expr || null;
}
// Returns all elements that match CSS selector {expr} as an array.
// Querying can optionally be restricted to {container}’s descendants
fun
Code Snippets
var sounds = {
block: new Audio('sounds/block.mp3'),
dodge: new Audio('sounds/dodged.mp3'),
sword: new Audio('sounds/sword.mp3')
};
sounds.sword.play(); // usagefunction createPlayer(name, maxLife) { // factory functions
var player = {
name: name,
life: maxLife,
deck: undefined // no cards yet
};
return player;
}
var player = createPlayer(undefined, 25); // usage
var computer = createPlayer("Hunter", 25);function createDeck(array) {
var cards = array; // private variable
return {
// removes the card off the top of the deck
draw: function () {
return cards.pop(); // the backside of the array the top of the deck
},
// returns the value of the card on the top of the deck
peek: function () {
return cards[cards.length - 1];
},
// returns the size of the deck
size: function () {
return cards.length;
},
// randomizes the order of the cards in the deck
shuffle: function () {
cards = shuffle(cards); // call your global shuffle function
},
// returns an array to be assigned to two players. Second array get's the extra card if the deck's length is odd
deal: function () {
var half = Math.floor(cards.length / 2);
var deck0 = createDeck(cards.slice(0, half));
var deck1 = createDeck(cards.slice(half));
return [deck0, deck1];
},
// returns a string of the deck
toString: function () {
return cards.toString();
}
};
}
var deck = createDeck([1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6, 7, 7, 8, 8];
deck.shuffle();
var hands = deck.deal();
player.deck = hands[0];
computer.deck = hands[1];function whoWon() {
"use strict";
var playerCard = player.deck.draw();
var computerCard = computer.deck.draw();
document.getElementById('computerDamageImage').setAttribute('src', 'images/empty.png');
document.getElementById('playerDamageImage').setAttribute('src', 'images/empty.png');
if (playerCard > computerCard) {
computerLife = computerLife - (playerTopCard - computerTopCard);
sounds.sword.play();
document.getElementById('computerDamageImage').setAttribute('src', 'images/damage.gif');
playerFeedback.innerHTML = computer.name + ' lost: ' + (playerTopCard - computerTopCard) + ' HP';
} else if (playerCard < computerCard) {
playerlife = playerlife - (computerTopCard - playerTopCard);
sounds.sword().play();
document.getElementById('playerDamageImage').setAttribute('src', 'images/damage.gif');
playerFeedback.innerHTML = playerName + ' lost: ' + (computerTopCard - playerTopCard) + ' HP';
} else {
sounds.block.play();
playerFeedback.innerHTML = 'It was a tie!';
}
checkIsGameOver();
}// Returns first element that matches CSS selector {expr}.
// Querying can optionally be restricted to {container}’s descendants
function $(expr, container) {
return typeof expr === "string"? (container || document).querySelector(expr) : expr || null;
}
// Returns all elements that match CSS selector {expr} as an array.
// Querying can optionally be restricted to {container}’s descendants
function $$(expr, container) {
return [].slice.call((container || document).querySelectorAll(expr));
}Context
StackExchange Code Review Q#128955, answer score: 6
Revisions (0)
No revisions yet.