-
Notifications
You must be signed in to change notification settings - Fork 660
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
Fundamentals - Beat That - 27-3_Karla #573
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the job that you did here Karla! A one more note other than the ones that I've given. Probably read a little bit more on the syntax of a for loop! In both cases it's preferable to use a for loop as opposed to a while loop here!
Other than that I'm excited to see your blackjack!
@@ -1,7 +1,7 @@ | |||
<!DOCTYPE html> | |||
<html> | |||
<head> | |||
<title>Coding Basics</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice that you've corrected Basics to fundamentals lol
<p> | ||
Create a two-digit number by selecting the order of your dice rolls. | ||
</p> | ||
<p>The player with the highest number wins! Good luck! <br /><br /></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that you've added this to help players know what the rules are
var GAME_STATE_COMPARE_SCORES = "GAME_STATE_COMPARE_SCORES"; | ||
var GAME_STATE_LEADERBOARD = "GAME_STATE_LEADERBOARD"; | ||
//first step of the game | ||
var gameState = GAME_STATE_DICE_ROLL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great use of constant variables!
|
||
// helper function = rollDice | ||
var rollDice = function () { | ||
console.log(`Control flow: start of rollDice()`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally for a "completed code" we don't add console logs anymore. It's always best practice to remove these on submission
while (counter < 2) { | ||
currentPlayerRolls.push(rollDice()); | ||
counter = counter + 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahem a for loop would be better here ahem
console.log(`running player 2 score is: ${runningPlayerScore2}`); | ||
j += 2; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels like a somewhat roundabout way of calculating the running sum of numbers, also if you're using i/j as an index here for a loop, it's ALWAYS best to keep the variable here, and not make it a global scope.
Now rather than having 2 separate loops here, we can make it more succinct with one loop through. As if let's say overallPlayersScore.length
is something VERY big, it will take very long to complete 2 loops as opposed to just one.
We can probably make the loop like so:
for(var i = 0; i<overallPlayersScore.length; i+=1){
if(i%2 == 0){
runningPlayerScore1 += overallPlayersScore[i]
} else {
runningPlayerScore2 += overallPlayersScore[i]
}
}
this way it's only one loop through, and we did all the things!
while (k < overallPlayersScore.length) { | ||
player1Scores.push(overallPlayersScore[k]); | ||
console.log(`summary player 1 scores: ${player1Scores}`); | ||
var sortedPlayer1Scores = player1Scores.slice().sort(function (a, b) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love that you have a array.slice().sort()
here!
...there's a new syntax to this tho, array.toSorted(function (a,b) {return b-a})
return b - a; | ||
}); | ||
l += 2; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same note to the looping above! might be more succinct to have one loop here.
Please fill out the survey before submitting the pull request. Thanks!
🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀
How many hours did you spend on this assignment?
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?
Comfort Level (1-5):
Completeness Level (1-5):
What did you think of this deliverable?
Is there anything in this code that you feel pleased about?
What's one aspect of your code you would like specific, elaborate feedback on?