-
Notifications
You must be signed in to change notification settings - Fork 27
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
Finished Word-Guess Game.--by Erika & Ari #4
base: master
Are you sure you want to change the base?
Conversation
Fixed loop on level option
missing color code--
Removed extra instance variable @correct_word
Word-Guess GameWhat We're Looking For
Good job overall! There's definitely some places where this code could be cleaned up, but in general this is a solid submission that meets the learning goals for this assignment. Let me know about questions on any of my comments below, and keep up the hard work! |
|
||
# make sure to not use the same word twice in game | ||
def dont_repeat(array) | ||
array.delete_at(rand(array.length)) |
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.
Watch your indentation here
_\|/_ | ||
|_____| | ||
| | | ||
|___|'''.red] |
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.
I suspect this is where your indentation gets messed up - Atom will want you to end the array on a new line.
|___|'''.red
]
end
# let the user know if they can't have more than one letter | ||
if (guess.length > 1) | ||
puts "\nYOU HAVE TO GUESS ONE LETTER AT A TIME.".red | ||
return promptUser |
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 portion, between lines 75 and 91, might be good to break out as a separate method, something like validate_guess
.
if (@correct_word_array.include?(guess)) #from index [0] to last index of word | ||
0.upto(@correct_word_array.length-1) { |index| | ||
if (@correct_word_array[index] == guess) | ||
@correct_guess[index] = guess |
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.
Please use do...end
for loops, rather than curly braces.
|
||
def promptUser | ||
unless (@correct_guess.include?("_") ) | ||
puts "\nYOU WIN! THE WORD WAS: #{@correct_word_array.join("")}".green |
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.
Ruby methods should be named using snake_case, so this one would be prompt_user
.
end | ||
} | ||
puts "SECERT WORD: #{@correct_guess.join(' ')}" | ||
promptUser # return to the prompt method to guess another letter |
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.
Here, promptUser
calls itself, creating a sort of loop. A method calling itself is a programming technique called recursion, which you'll learn about in CS Fun in a few months.
Recursion is a powerful tool that can elegantly solve many problems. However, in this case I think a while
loop might be a better choice, if only because it makes it immediately obvious to the reader that this code might execute many times.
puts "SECERT WORD: #{@correct_guess.join(' ')}" | ||
promptUser # return to the prompt method to guess another letter | ||
pedal_remove # remove petals from the picture | ||
else |
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.
I don't think this call to pedal_remove
is actually doing anything.
# if the users letter is in the word | ||
if (@correct_word_array.include?(guess)) #from index [0] to last index of word | ||
0.upto(@correct_word_array.length-1) { |index| | ||
if (@correct_word_array[index] == guess) |
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.
I agree that this logic is a little dense. One observation is that we don't need the check on line 96, because if the correct word does not include the guess, then the if statement on line 98 will never go off.
We could simplify the whole section by rewriting it as:
letter_found = false
@correct_word_array.each_with_index do |letter, index|
if letter == guess
@correct_guess[index] = guess
letter_found = true
end
end
if letter_found
puts "SECERT WORD: #{@correct_guess.join(' ')}"
promptUser # return to the prompt method to guess another letter
else
# ... the rest of this method, starting on line 105 ...
end
Word Guess
Congratulations! You're submitting your assignment.
Comprehension Questions