Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beat That #138

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Beat That #138

wants to merge 2 commits into from

Conversation

imkenhere
Copy link

Please fill out the survey before submitting the pull request. Thanks!

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

How many hours did you spend on this assignment?

2 days 😅

Please fill in one error and/or error message you received while working on this assignment.
script errors

What part of the assignment did you spend the most time on?
reordering the code to make it work. As whenever I want to add a new functionality to the game, I am unsure of where to place it.

Comfort Level (1-5):
2

Completeness Level (1-5):
4

What did you think of this deliverable?
Quite difficult compared to the scissors paper stone game.

Is there anything in this code that you feel pleased about?

return `It is a draw! Have another go!`;
}
}
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline required at end of file but not found.

return `Congratulations Player 2! You have won! <br> Player 2 had ${player2Order} and Player 1 had ${player1Order} <br><br> Leaderboard: <br> Player 1: ${player1Win} <br> vs <br> Player 2: ${player2Win} <br> <br> Press submit to play again!`;
}
if (
player2Order = player1Order
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected assignment within an 'if' statement.

activePlayer = player2;
gameMode = diceRollMode;
return `${gameDiceOrder} <br> <br> Now it is player 2's turn! <br> <br> Player 2, press submit to roll.`;
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary 'else' after 'return'.

if (gameMode == diceOrderMode) {
var gameDiceOrder = diceOrder(input);
if (input != 1 && input != 2){
return `Please input 1 or 2.`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon.

}
if (gameMode == diceOrderMode) {
var gameDiceOrder = diceOrder(input);
if (input != 1 && input != 2){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space before opening brace.

@codeclimate
Copy link

codeclimate bot commented Aug 2, 2021

Code Climate has analyzed commit f1b91ad and detected 15 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 6
Bug Risk 5
Clarity 4

Note: there are 9 critical issues.

View more on Code Climate.

Copy link

@eddiejpot eddiejpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good job ! good use of helper functions :)

Comment on lines +2 to +6
var diceRollMode = "dice rolling!";
var diceOrderMode = "choose the order of dice";
var beatThatWinner = "comparing the numbers";
//default game mode
var gameMode = diceRollMode;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice breakdown of gameModes here!

Would use scream case here to better separate this from the other variables
e.g.

DICEROLL_MODE = 'diceRolling'
gameMode = DICEROLL_MODE 

Comment on lines +28 to +37
var dicePush = function () {
var dicePush = [diceRoll(), diceRoll()];

// Checking if player is 1 or 2, then pushing the dice numbers into the array
if (activePlayer == player1 && gameMode == diceRollMode) {
player1Dice = dicePush;
} else if (activePlayer == player2) {
player2Dice = dicePush;
}
return dicePush;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually don't assign arrays like this. It's better to do the push method
Reason being this idea of reference vs primitive types. - I'll run through this in class !

Watch this video to find out more :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants