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

Pipes - Kate Evans-Spitzer & Angela Wilson - Word-Guess #3

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

Conversation

Guribot
Copy link

@Guribot Guribot commented Aug 18, 2017

Word Guess

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
How do you feel you and your partner did in sharing responsibilities? We think we did well!
Describe an instance where you used a method for something to encapsulate the functionality within your class. What does it do? What are its inputs and outputs? Our game has a method called "add_letter" that takes in the user's guess, finds all indices of this letter within the original word, and replaces the appropriate placeholders ("_") with the correct letter.
Describe an instance where you used a local variable instead of an instance variable. Why did you make that choice? The above "add_letter" method uses a local variable called letter_location. It's an array that contains all found indices of the letter being guessed. We used a local variable for that because the variable is only used within the scope of the method.
What code, if any, did you feel like you were duplicating more than necessary? Our ASCII art is made up of multiple music notes that disappear as the user runs out of tries. We couldn't find a way to print multi-line ASCII art one at a time, so some of those images appear more than once in the code.
Is there a specific piece of code you'd like feedback on? We spent a long time trying to wrap our heads around the variables "user_has_won" and "user_has_lost", especially trying to figure out if it's necessary to include the variable definitions inside of the loop (lines 161 & 162). We'd be curious to know if there is a better way to handle or understand these.

@droberts-sea
Copy link

droberts-sea commented Aug 22, 2017

Word-Guess Game

What We're Looking For

Feature Feedback
Baseline
Regular Commits with meaningful commit messages. yes
Readable code with consistent indentation. yes
Answered comprehension questions yes
Product Functionalities
Created a Class to encapsulate game functionality. yes
Used methods to DRY up your code. yes
Created instance variables & local variables where appropriate. yes
Used Arrays to store lists of letters guessed. yes
Used variables & random numbers to allow the game to function with multiple words, no hard-coded answers. yes
Programmed "defensively" to detect errors in user input. no - Guessing the same thing multiple times was guarded against, but I was able to guess "letters" such as %, test and 37

Great work overall. Code is well-organized and easy to read. I've got a few specific comments below, but in general I'm quite happy with this submission.

user_has_won = !(game_board.blank.include?"_")
user_has_lost = game_board.tries_remaining == 0

until (user_has_won || user_has_lost)

Choose a reason for hiding this comment

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

Since you make these calculations for user_has_won and user_has_lost multiple times, they might be a good candidate for an instance method on Board.

puts "Please guess a letter: "
guess = gets.chomp.upcase
if game_board.letters_guessed.include?guess
puts "You've already guessed that!"

Choose a reason for hiding this comment

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

This logic all looks good, but the code might be a little cleaner if it were all wrapped up into its own method. Maybe call it something like take_turn.

def initialize_word_bank
return ["ear",
"heart",
"irritate",

Choose a reason for hiding this comment

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

I love that you've separated your words out into a separate file. This makes it very clear which part of the program is which, and the reader doesn't have to scroll through a hundred lines of words at the top of your file.

def display
output = ""
output += @tries_pictures[@tries_remaining-1] + "\n"
output += @blank + "\n"

Choose a reason for hiding this comment

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

I like that you've rolled all this up into an instance method. Very concise.

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