Skip to content

27-2 Lena's Blackjack project #493

Open
purplekiwifruit wants to merge 3 commits intorocketacademy:mainfrom
purplekiwifruit:main
Open

27-2 Lena's Blackjack project #493
purplekiwifruit wants to merge 3 commits intorocketacademy:mainfrom
purplekiwifruit:main

Conversation

@purplekiwifruit
Copy link
Copy Markdown

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

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

How many hours did you spend on this assignment? about 8-10hrs

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

What part of the assignment did you spend the most time on? Game logic and DOM manipulation

Comfort Level (1-5): 4

Completeness Level (1-5): 5

What did you think of this deliverable? Super glad that I'm able to create a game webpage.

Is there anything in this code that you feel pleased about? The DOM manipulation makes the webpage more interactive.

What's one aspect of your code you would like specific, elaborate feedback on?

Copy link
Copy Markdown

@zephaniahong zephaniahong left a comment

Choose a reason for hiding this comment

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

Hi Lena! An awesome job completing this project using DOM Manipulation. Overall, I think you did a really good job! All my comments are most nit picks on how it could have been better. The main room for improvement would just be spotting repeated logic and thinking of how you can refactor those into helper functions to make your code more readable.
Congrats on completing the course!!!

var computerScore = 0;

//defind helper functions
function checkBlackJacket(num1, num2) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It will be more ideal for this function to return a boolean instead of updating the score variable.

return score;
}

function compareScore(num1, num2) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is good!

}

playButton.addEventListener("click", () => {
for (var i = 0; i < 2; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the next 3 lines seem to be repeated at a couple of places. It's a hint that you probably want a function for it

playerScore = checkBlackJacket(playerCards[0].value, playerCards[1].value);

if (playerScore == 21) {
playerScore = playerScore;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this line seems redundant

computerCards[0].value,
computerCards[1].value
);
if (computerScore == 21) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this if else statement also seems to be repeated.. a function would be helpful(:

});

hitButton.addEventListener("click", () => {
var card = shuffledDeck.pop();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same here. These lines are repeated on lines 174 as well. Use a function

});

standButton.addEventListener("click", () => {
if (computerScore < 17) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this logic also seems to be repeated on lines 173.

outputMessageElement.innerHTML = ` Computer Points: ${computerScore}<br>Player Points: ${playerScore}<br> ${winner} `;
});

restartButton.addEventListener("click", () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is nice!

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