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

Homework-04 #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

AtleLohrmann
Copy link

I have not done the optional work - did not notice it before it had been uploaded. I do need to work more on jQuery so I will redo some of the earlier "code-alongs" to practice

Copy link

@aghaffar570 aghaffar570 left a comment

Choose a reason for hiding this comment

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

@AtleLohrmann Great work on the rock-scissor-paper game!! 👍
Excellent job on the commenting and separation of logic using functions. 💯


// If the user clicks "rock":
$('#rock').click(() => {
let userInput = $('#rock').text()

Choose a reason for hiding this comment

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

@AtleLohrmann you can also grab the text from $(this) because this would refer to the object clicked.
Also, because we know which button was clicked, you can use an inline string with the respective button's value.

check out this approach

// generate a random integer [0:2]
let randomInt = getRandomInt(3)
if (randomInt===0) {
choice='rock'

Choose a reason for hiding this comment

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

@AtleLohrmann this is a functionally scoped var declaration. just curious why you switched from es6 to es5. It still works, so no worries there.

Comment on lines +102 to +104
} else {
alert('something wrong with the getMachineInput routine [0:2]:' + randomInt)
}

Choose a reason for hiding this comment

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

@AtleLohrmann Great use case for error handling. However, this statement would never run because of the randomly generated number.

Comment on lines +9 to +10
let machineInput=NaN
let userInput=NaN

Choose a reason for hiding this comment

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

@AtleLohrmann The input corresponds to a string when it's assigned, so having either an empty string or null would be a better candidate for a default value. However, Great job on experimenting with other data types for default values.

// Extra info for the user for clarity and also debugging -- a little awkward but it works
if (winner===1) {gwin="User"} else {gwin="Bot"} //transfer winner to text that we can use
$('#status').html("Computer played " + '<u>'+machineInput+'</u>' + '.' + '\n'
+ "You played " + '<u>'+userInput+'</u>'+'.' + '</br>' + gwin + " Won" )

Choose a reason for hiding this comment

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

@AtleLohrmann the UI does not account for tied games

check solution here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants