Reputation: 37
I am currently studying JS myself.
And make the pig-game which is project of the course that I watched.
I'm always curious how can I improve my code but I got no idea where do I begin.
Is there some principle that I can improve any code?
If there's a way, Could you let me know?
Thanks!
https://github.com/wonkooklee/pig-game
result : https://wonkooklee.github.io/pig-game/
below is main functions
document.querySelector('.btn-roll').addEventListener('click', function() {
if (gamePlaying) {
dice = Math.floor(Math.random() * 6) + 1;
diceDOM.style.display = 'block';
diceDOM.src = `dice-${dice}.png`;
if (dice === 6 && lastDice === 6) {
// Player looses score
scores[activePlayer] = 0;
document.getElementById(`score-${activePlayer}`).textContent = '0';
nextPlayer();
} else if (dice !== 1 && dice !== 6) {
roundScore += dice;
document.getElementById(`current-${activePlayer}`).textContent = roundScore;
lastDice = 0;
} else if (dice === 6) {
roundScore += dice;
document.getElementById(`current-${activePlayer}`).textContent = roundScore;
lastDice = dice;
} else {
nextPlayer();
}
}
});
document.querySelector('.btn-hold').addEventListener('click', function() {
if (gamePlaying) {
scores[activePlayer] += roundScore;
document.getElementById(`score-${activePlayer}`).textContent = scores[activePlayer];
let input = document.getElementById('scoreSet').value;
let winningScore;
if (isNaN(input) === false) {
winningScore = input;
} else {
document.getElementById('scoreSet').value = '100';
}
if (scores[activePlayer] >= winningScore) {
document.getElementById(`name-${activePlayer}`).textContent = 'WINNER!';
document.querySelector(`.player-${activePlayer}-panel`).classList.add('winner');
document.querySelector(`.player-${activePlayer}-panel`).classList.remove('active');
diceDOM.style.display = 'none';
gamePlaying = false;
} else {
nextPlayer();
}
}
});
Upvotes: 0
Views: 75
Reputation: 710
Martin Fowler wrote a great book "Refactoring". Besides, Fowler has a great blog Refactoring.com, where you can find a lot of information about refactoring with examples in Javascript. I'm not strong in Javascript, but can let you some advices about your code.
1.Simplify Conditional Logic
For example like this:
if (dice === 6 && lastDice === 6) {
// Player looses score
scores[activePlayer] = 0;
document.getElementById(`score-${activePlayer}`).textContent = '0';
nextPlayer();
return;
}
if (dice !== 1 && dice !== 6) {
roundScore += dice;
document.getElementById(`current-${activePlayer}`).textContent = roundScore;
lastDice = 0;
return;
}
if (dice === 6) {
roundScore += dice;
document.getElementById(`current-${activePlayer}`).textContent = roundScore;
lastDice = dice;
return;
}
nextPlayer();
2.Delete duplicated code and extract function
For example
function someFunctionName(diffRoundScore, lastDiceValue){
roundScore += diffRoundScore;
document.getElementById(`current-${activePlayer}`).textContent = roundScore;
lastDice = lastDiceValue;
}
if (dice !== 1 && dice !== 6) {
someFunctionName(dice, 0);
return;
}
if (dice === 6) {
someFunctionName(dice, dice);
return;
}
3.Change check "dice for 6" to function
function isDiceEqualsSix { return dice === 6};
if (isDiceEqualsSix && lastDice === 6) {
// Player looses score
scores[activePlayer] = 0;
document.getElementById(`score-${activePlayer}`).textContent = '0';
nextPlayer();
return;
}
if (dice !== 1 && !isDiceEqualsSix) {
someFunctionName(dice, 0);
return;
}
if (isDiceEqualsSix) {
someFunctionName(dice, dice);
return;
}
4.Change "6" to a constant variable or a function
const LIMIT_SCORE = 6;
function isDiceEqualsSix { return dice === LIMIT_SCORE};
if (isDiceEqualsSix && lastDice === LIMIT_SCORE) {
// Player looses score
scores[activePlayer] = 0;
document.getElementById(`score-${activePlayer}`).textContent = '0';
nextPlayer();
return;
}
I hope I helped you.
Upvotes: 1