Basics Beat That Submission: Elky #577
Conversation
There was a problem hiding this comment.
Hey Elky! This is a great attempt on the second project! I know that you tried your best with this, but here's a couple of notes:
- Familiarize yourself with arrays, I don't see that you're using any arrays here. There are cases in which it's preferable to use arrays than not here.
- The main function is rather slightly bulky. Maybe it's best to have smaller helper functions to help you alleviate the bulk from the main function.
All in all, it's a great attempt at the beat that project and I am excited to see what you can do in your blackjack project!
| @@ -1,7 +1,7 @@ | |||
| <!DOCTYPE html> | |||
| <html> | |||
| <head> | |||
| <title>Coding Basics</title> | |||
| <title>Beat That!</title> | |||
There was a problem hiding this comment.
Love that you changed the title here!
| @@ -40,7 +40,7 @@ | |||
| } | |||
|
|
|||
| #output-div { | |||
| background-color: lightgrey; | |||
| background-color: rgb(254, 255, 243); | |||
There was a problem hiding this comment.
It's great that you're playing with rgb for the display! This adds a great deal of personal touch to your code!
| <button id="submit-button">Submit</button> | ||
| <p>Current state:</p> | ||
| <div id="output-div"></div> | ||
| </body> |
There was a problem hiding this comment.
Generally, you shouldn't put a <body> tag within another <body> tag. a standard <p> tag or a <div> tag would suffice here.
| var player1DiceRoll1, player1DiceRoll2; | ||
| var player2DiceRoll1, player2DiceRoll2; | ||
|
|
||
| var player1DiceOrder, player2DiceOrder; |
There was a problem hiding this comment.
while this method of declaring multiple variables work, I'd much prefer if you would just have them line by line as opposed to them being in one line. It's generally more readable that way.
var player1DiceOrder
var player2DiceOrder
| var currentGameMode = playerToRollDice; | ||
|
|
||
| var player1DiceRoll1, player1DiceRoll2; | ||
| var player2DiceRoll1, player2DiceRoll2; |
There was a problem hiding this comment.
I'd probably explore the usage of an array here. Although this works, it gets harder to scale the game IF you decide to make a game with multiple dice like say, 5 dice, 6 dice.
you'd have to have the same number of variables with the dice. and it's not super ideal.
| player1DiceOrder = "" + player1DiceRoll1 + player1DiceRoll2; | ||
| } else { | ||
| player1DiceOrder = "" + player1DiceRoll2 + player1DiceRoll1; | ||
| } |
There was a problem hiding this comment.
This is a novel way of combining the numbers! adding a string to a number would cast that number as a string as opposed to a number, hence why this works.
Also, it might be best to add an input validation somewhere here as this would mean that if you'd have an input other than 1 it will always default to the second choice. i.e. if let's say my input is 3 it'll go to the else block, even when my input should not be a valid input like say "what's up" it will still go to the else block.
Please fill out the survey before submitting the pull request. Thanks!
🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀
How many hours did you spend on this assignment?
Over 10++ hours 🥲
Please fill in one error and/or error message you received while working on this assignment.
Returned with an empty state as I was unable to change the game modes.
What part of the assignment did you spend the most time on?
Figuring out how to compare the two scores and also pushing to Git. Took an hour plus to figure out pushing to Git as I had a few errors.
Comfort Level (1-5): 3
Completeness Level (1-5): 3
What did you think of this deliverable?
I would like to be able to spend more time to cover more scenarios (e.g. if user doesn't input 1 or 2, what message will they see)
Is there anything in this code that you feel pleased about?
At least I'm done with basics!
What's one aspect of your code you would like specific, elaborate feedback on?