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

Gloria-beatthat #148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Gloria-beatthat #148

wants to merge 1 commit into from

Conversation

Gloriahe
Copy link

@Gloriahe Gloriahe commented Aug 6, 2021

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

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

How many hours did you spend on this assignment?
5

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

What part of the assignment did you spend the most time on?
understanding the logic

Comfort Level (1-5):
2

Completeness Level (1-5):
2

What did you think of this deliverable?
challenging

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

rollDice2 = randomDigit();
myOutputValue = `Player 2, your Dice 1 is ${rrollDice1} and Dice 2 is ${rollDice2}. <br><br> Please indicate either '1' or '2' to choose the order of your dice.<br><br> User with the highest number will win. Good luck!`;
gameMode = "pick_dice_order";
return myOutputValue;
Copy link

Choose a reason for hiding this comment

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

Parsing error: 'return' outside of function

Choose a reason for hiding this comment

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

Please delete unnecessary code

}
if (gameModeOrder == "pick_dice_order") {
if (currentPlayer == 1) {
if (input == "1") {
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

return myOutputValue;
};
if (gameMode == "start_the_game") {
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

gameMode = "start_the_game";
currentPlayer = 2;
} else if (currentPlayer == 2) {
if (input == "1") {
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

var main = function (input) {
var myOutputValue = 'hello world';
var myOutputValue = "";
if (gameMode == "start_the_game") {
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Aug 6, 2021

Code Climate has analyzed commit f7f4669 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Duplication 4
Style 1

Note: there is 1 critical issue.

View more on Code Climate.

@Gloriahe Gloriahe changed the title beatthat Gloria-beatthat Aug 6, 2021
if (gameMode == "start_the_game") {
diceRoll1 = genDiceRoll();
diceRoll2 = genDiceRoll();
myOutputValue = `Player 1, your Dice 1 is ${diceRoll1} and <br> Dice 2 🎲is ${diceRoll2}. <br><br> Please indicate either '1' or '2' to choose the order of your dice.<br><br> User with the highest number will win. Good luck!`;

Choose a reason for hiding this comment

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

Good work crafting an output that clearly guides the player on what to do next!

myOutputValue = `You have chosen Dice 1 and your number is ${diceRoll1}${diceRoll2}.`;
}
} else if (input == "2") {
myOutputValue = `You have chosen Dice 2 and your number is ${diceRoll2}${diceRoll1}.`;

Choose a reason for hiding this comment

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

There should be something like player1Choice = ${diceRoll2}${diceRoll1} here as well, similar to line 36.

console.log("player2Choice");
myOutputValue = `You have chosen Dice 1 and your number is ${diceRoll1}${diceRoll2}.`;
} else if (input == "2") {
myOutputValue = `You have chosen Dice 2 and your number is ${diceRoll2}${diceRoll1}.`;

Choose a reason for hiding this comment

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

There should be something like player1Choice = ${diceRoll2}${diceRoll1} here as well, similar to line 47.

} else if (player1Choice < player2Choice) {
myOutputValue = `Player 1, your score is ${player1Choice} and Player 2, your score is ${player2Choice}. <br><br>Player 2, you win! Yay`;
}
return myOutputValue;

Choose a reason for hiding this comment

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

Do consider a tie condition too!

Copy link

@lightweightcoder lightweightcoder 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 at attempting the exercise! I think you nailed the naming convention for variables and made good use of return statements to prevent the need for too many if-else conditions.

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