BeatThatProject (Winston) #141
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| if (gameMode == "WHO WON") { | ||
| if (Number(finalValuePlayerOne) > Number(finalValuePlayerTwo)) { | ||
| return `Player One's final value of ${finalValuePlayerOne} is larger than Player Two's final value of ${finalValuePlayerTwo}. <br><br>Player One wins!`; | ||
| } else { |
There was a problem hiding this comment.
Unnecessary 'else' after 'return'.
|
|
||
| if (gameMode == "PLAYER TWO GUESS MODE") { | ||
| var firstValue = input; | ||
| var myOutputValue = ``; |
There was a problem hiding this comment.
'myOutputValue' is already defined.
| } | ||
|
|
||
| if (gameMode == "PLAYER TWO GUESS MODE") { | ||
| var firstValue = input; |
There was a problem hiding this comment.
'firstValue' is already defined.
|
|
||
| gameMode = "PLAYER ONE GUESS MODE"; | ||
|
|
||
| return (myOutputValue = `Player One has rolled ${playerOneDiceArray[0]} and ${playerOneDiceArray[1]}. <br> To choose ${playerOneDiceArray[0]} as the first value, submit "1" . <br> To choose ${playerOneDiceArray[1]} as the first value, submit "2".`); |
There was a problem hiding this comment.
'myOutputValue' was used before it was defined.
|
|
||
| gameMode = "PLAYER ONE GUESS MODE"; | ||
|
|
||
| return (myOutputValue = `Player One has rolled ${playerOneDiceArray[0]} and ${playerOneDiceArray[1]}. <br> To choose ${playerOneDiceArray[0]} as the first value, submit "1" . <br> To choose ${playerOneDiceArray[1]} as the first value, submit "2".`); |
There was a problem hiding this comment.
Return statement should not contain assignment.
|
Code Climate has analyzed commit 08e561a and detected 12 issues on this pull request. Here's the issue category breakdown:
Note: there are 6 critical issues. View more on Code Climate. |
| //global variables | ||
| var gameMode = "PLAYER ONE ROLL MODE"; |
There was a problem hiding this comment.
Would be good to comment out all the gameModes here for clarity :)
| var diceRoll1 = rollDice(); | ||
| var diceRoll2 = rollDice(); | ||
| var diceRoll3 = rollDice(); | ||
| var diceRoll4 = rollDice(); |
There was a problem hiding this comment.
Interesting choice of rolling the dice on page load.
However might be better to only rolling the dice when needed. Reason being our code has to be flexible for any changes (e.g. what if the player needs to roll 3 dices, what if there are more than 2 players)
| if (gameMode == "PLAYER ONE GUESS MODE") { | ||
| var firstValue = input; | ||
| var myOutputValue = ``; | ||
| if (firstValue == "1") { | ||
| finalValuePlayerOne = `${playerOneDiceArray[0]}${playerOneDiceArray[1]}`; | ||
| myOutputValue += `Player One's final value is ${finalValuePlayerOne}.`; | ||
| } else if (firstValue == "2") { | ||
| finalValuePlayerOne = `${playerOneDiceArray[1]}${playerOneDiceArray[0]}`; | ||
| myOutputValue += `Player One's final value is ${finalValuePlayerOne}.`; | ||
| } | ||
| gameMode = "PLAYER TWO ROLL MODE"; | ||
| return myOutputValue; | ||
| } |
There was a problem hiding this comment.
You'll notice that in the the GUESS MODE gameMode. Player 1 and 2 's code are almost identical.
This is usually a signal to decompose the logic here into a separate function that can be used for player1 and player2.
Take a look at the reference solution for this exercise and how multiple helper functions are used
reference
Please fill out the survey before submitting the pull request. Thanks!
🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀
How many hours did you spend on this assignment?
4
Please fill in one error and/or error message you received while working on this assignment.
There was no output in between "submit" button presses.
What part of the assignment did you spend the most time on?
I spent the most time on creating the logic.
Comfort Level (1-5):
4
Completeness Level (1-5):
4
What did you think of this deliverable?
It is a good reflection of what was taught.
Is there anything in this code that you feel pleased about?
It works well.