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

beatthat #152

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

beatthat #152

wants to merge 1 commit into from

Conversation

hell0kj
Copy link

@hell0kj hell0kj commented Sep 3, 2021

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

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

How many hours did you spend on this assignment?
3hrs
Please fill in one error and/or error message you received while working on this assignment.
unable to string the 2 dice rolls together.
What part of the assignment did you spend the most time on?
stringing the 2 numbers together
Comfort Level (1-5):
3
Completeness Level (1-5):
3
What did you think of this deliverable?
Manageable.
Is there anything in this code that you feel pleased about?
NA

"<br>" +
player1Statement +
". <br>" +
player2Statement +
Copy link

Choose a reason for hiding this comment

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

'player2Statement' is not defined.

"Current Game Mode: " +
gameMode +
"<br>" +
player1Statement +
Copy link

Choose a reason for hiding this comment

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

'player1Statement' is not defined.

if (player2 > player1) {
message = player2Name + " is leading!";
}
return (myOutputValue =
Copy link

Choose a reason for hiding this comment

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

'myOutputValue' is not defined.

if (player2 > player1) {
message = player2Name + " is leading!";
}
return (myOutputValue =
Copy link

Choose a reason for hiding this comment

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

Return statement should not contain assignment.

player2Storage = sequence4;
player2 = player2 + player2Storage;
}
player2Statement =
Copy link

Choose a reason for hiding this comment

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

'player2Statement' is not defined.

@codeclimate
Copy link

codeclimate bot commented Sep 3, 2021

Code Climate has analyzed commit 6c9d3e9 and detected 69 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 13
Duplication 4
Bug Risk 51
Clarity 1

Note: there are 52 critical issues.

View more on Code Climate.

Comment on lines +38 to +49
// Enter Player 1 Name
if (player1Name == "Player1Name") {
var playerName = enterPlayerName(input);
player1Name = playerName;
return player1Name + " has joined the game";
}
// Enter Player 2 Name
if (player2Name == "Player2Name") {
var playerName = enterPlayerName(input);
player2Name = playerName;
return player2Name + " has joined the game";
}

Choose a reason for hiding this comment

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

smart way of getting players name but it would be better if there is an put invalidation

Comment on lines +51 to +60
if (
input == "Normal" ||
input == "Lowest" ||
input == "Auto" ||
playerTurn == "End"
) {
gameMode = input;
playerTurn = "Player 1";
return (myOutputValue = input + " has been selected. Lets begin!");
} else myOutputValue = "Choose only Normal, Lowest or Auto~";

Choose a reason for hiding this comment

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

good input validation

Comment on lines +67 to +83
// player1 turn
if (playerTurn == "Player 1") {
var diceRoll1 = randomDiceRoll();
var diceRoll2 = randomDiceRoll();
dice1 = diceRoll1; // storing player 1 numbers into global
dice2 = diceRoll2;
myOutputValue =
player1Name +
" has rolled " +
dice1 +
" and " +
dice2 +
". <br> Select the sequence.";
sequenceToggle = "ON";
playerTurn = "Player 1 Sequence Toggle";
return myOutputValue;
}

Choose a reason for hiding this comment

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

player 1 roll dice and player 2 roll dice are the same logic. You could refactor it into a function and reuse it instead of rewriting the same logic

Comment on lines +85 to +99
if (sequenceToggle == "ON" && playerTurn == "Player 1 Sequence Toggle") {
if (input == "1") {
var sequence1 = selectSequence1(dice1, dice2);
player1Storage = sequence1;
player1 = player1 + player1Storage;
sequenceToggle = "OFF";
playerTurn = "Player 2";
}
if (input == "2") {
var sequence2 = selectSequence2(dice2, dice1);
player1Storage = sequence2;
player1 = player1 + player1Storage;
sequenceToggle = "OFF";
playerTurn = "Player 2";
}

Choose a reason for hiding this comment

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

same as above , repeated code could be refactor with function helper and reuse it for both player 1 and player 2

Comment on lines +102 to +134
if (playerTurn == "Player 2") {
var diceRoll1 = randomDiceRoll();
var diceRoll2 = randomDiceRoll();
dice1 = diceRoll1; // storing player 2 numbers into global
dice2 = diceRoll2;
myOutputValue =
player2Name +
" has rolled " +
dice1 +
" and " +
dice2 +
". <br> Select the sequence.";
sequenceToggle = "ON";
playerTurn = "Player 2 Sequence Toggle";
return myOutputValue;
}
// player 2 selection
if (sequenceToggle == "ON" && playerTurn == "Player 2 Sequence Toggle") {
if (input == "1") {
var sequence1 = selectSequence1(dice1, dice2);
player2Storage = sequence1;
player2 = player2 + player2Storage;
sequenceToggle = "OFF";
playerTurn = "Player 1";
}
if (input == "2") {
var sequence2 = selectSequence2(dice2, dice1);
player2Storage = sequence2;
player2 = player2 + player2Storage;
sequenceToggle = "OFF";
playerTurn = "Player 1";
}
}

Choose a reason for hiding this comment

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

same as above

Comment on lines +202 to +261
if (playerTurn == "Player 2") {
var diceRoll1 = randomDiceRoll();
var diceRoll2 = randomDiceRoll();
dice1 = diceRoll1; // storing player 2 numbers into global
dice2 = diceRoll2;
myOutputValue =
player2Name +
" has rolled " +
dice1 +
" and " +
dice2 +
". <br> Select the sequence.";
sequenceToggle = "ON";
playerTurn = "Player 2 Sequence Toggle";
return myOutputValue;
}
// player 2 selection
if (sequenceToggle == "ON" && playerTurn == "Player 2 Sequence Toggle") {
if (input == "1") {
var sequence1 = selectSequence1(dice1, dice2);
player2Storage = sequence1;
player2 = player2 + player2Storage;
sequenceToggle = "OFF";
playerTurn = "Player 1";
}
if (input == "2") {
var sequence2 = selectSequence2(dice2, dice1);
player2Storage = sequence2;
player2 = player2 + player2Storage;
sequenceToggle = "OFF";
playerTurn = "Player 1";
}
}
// compare winners and message
var message = "";
if (player1 == player2) {
message = "Score is tied now";
}
if (player1 < player2) {
message = player1Name + " is leading the losing race!";
}
if (player2 < player1) {
message = player2Name + " is leading the losing race!";
}
return (
"Current Game Mode: " +
gameMode +
"<br>" +
message +
"<br>" +
player1Name +
" score now is " +
player1 +
"<br>" +
player2Name +
" score now is " +
player2 +
"<br> Next Round."
);
}

Choose a reason for hiding this comment

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

same as above

Comment on lines +273 to +280
if (dice1 > dice2) {
var sequence1 = selectSequence1(dice1, dice2);
player1Storage = sequence1;
player1 = player1 + player1Storage;
} else {
var sequence2 = selectSequence2(dice2, dice1);
player1Storage = sequence2;
player1 = player1 + player1Storage;

Choose a reason for hiding this comment

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

smart way of getting the sequence

Comment on lines +274 to +278
var sequence1 = selectSequence1(dice1, dice2);
player1Storage = sequence1;
player1 = player1 + player1Storage;
} else {
var sequence2 = selectSequence2(dice2, dice1);

Choose a reason for hiding this comment

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

selectSequence1 and selectSequence2 are using the same logic , reuse it instead of creating a new function that does the same thing

Comment on lines +292 to +304
var diceRoll3 = randomDiceRoll();
var diceRoll4 = randomDiceRoll();
dice3 = diceRoll3; // storing player 2 numbers into global
dice4 = diceRoll4;
if (dice3 > dice4) {
var sequence3 = selectSequence1(dice3, dice4);
player2Storage = sequence3;
player2 = player2 + player2Storage;
} else {
var sequence4 = selectSequence2(dice4, dice3);
player2Storage = sequence4;
player2 = player2 + player2Storage;
}

Choose a reason for hiding this comment

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

same as above

Comment on lines +316 to +324
if (player1 == player2) {
message = "Score is tied now";
}
if (player1 > player2) {
message = player1Name + " is leading!";
}
if (player2 > player1) {
message = player2Name + " is leading!";
}

Choose a reason for hiding this comment

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

repeated code could be refactor and reuse for all the game mode

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