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

Word Guess - Brandy and Angela - Octos #9

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

Conversation

knockknockhusthere
Copy link

@knockknockhusthere knockknockhusthere commented Feb 14, 2018

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 shared responsibilities well, but did things together most of the time instead of separating roles and driver and navigator. It was hard to switch appropriately bc we needed to work together to come up with most things.
For each partner what parts of the project did you find challenging? It was difficult to add the user interface at the end because it broke our functionality and we couldn't determine why. It was also challenging to determine how we should use classes.
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? We use a method in the class GameBehavior to replace the underscores with the correctly guessed letters. The method, replace underscores, takes in the letter the user guesses as well as the word. It then outputs an instance variable called guess_status. This instance variable is the updated string with underscores and correctly guessed letters.
Describe an instance where you used a local variable instead of an instance variable. Why did you make that choice? When evaluating whether the user won or lost the game, we used a local variable status. This variable was only needed within the related if/else statement, but no where else. As a result an instance variable was unnecessary.
What code, if any, did you feel like you were duplicating more than necessary? We did a good job not repeating code overall. That being said, we re-call methods when running through the user interface. We do so because the behavior is slightly different the first time we ask the user to enter a letter. There may be a better way to loop through these actions.
Is there a specific piece of code you'd like feedback on? In the UI section, we use a counter to determine if it's the user's first time guessing. We briefly discussed a better way to do this, but were not able to implement something. Do you have more feedback about how to do this without a counter?

@CheezItMan
Copy link

Word-Guess Game

What We're Looking For

Feature Feedback
Baseline
Regular Commits with meaningful commit messages. Check, good commit messages
Readable code with consistent indentation. Check
Answered comprehension questions Check, see my in-code notes.
Product Functionalities
Created a Class to encapsulate game functionality. Check
Used methods to DRY up your code. Check
Created instance variables & local variables where appropriate. Check, although see my in-code notes
Used Arrays to store lists of letters guessed. Check
Used variables & random numbers to allow the game to function with multiple words, no hard-coded answers. Check
Programmed "defensively" to detect errors in user input. Check, although it doesn't prevent me from guessing the same letter over and over again.
Summary Nicely done, I left some in-code notes for you, but the project works and the theme certainly fits! You hit all the requirements, plus a few extras. You did leave a syntax error in your code that I had to fix before running it. You hadn't given colorize an argument.

# start body

# intro
puts "Welcome to Val's Word Guess!\n\n".colorize

Choose a reason for hiding this comment

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

This is a bug? Did you test it?

def user_choose_letter
puts "\n\nChoose a letter: "
user_guess = gets.chomp
until user_guess =~ /^[a-zA-Z]$/

Choose a reason for hiding this comment

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

I suggest also testing to make sure the user hasn't guessed that letter yet.

end

def create_underscores
word_length = @word.instance_variable_get(:@word_array).length

Choose a reason for hiding this comment

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

If you're going to get the word's word_array instance variable, use an attr_reader in the Word class. In general, never use instance_variable_get. That bypasses the public interface of the class.

Also you're never using word_length

@guess_status = []
end

def update_guess_counter(true_false)

Choose a reason for hiding this comment

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

What is true_false supposed to represent. This isn't a great variable name.

end

def update_guess_counter(true_false)
@guess_counter -= 1 if true_false == false

Choose a reason for hiding this comment

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

Instead of true_false == false You can just do if !true_false


game.print_ascii_art
puts "\n"
underscores = game.create_underscores

Choose a reason for hiding this comment

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

You could move game.create_underscores to before the loop starts, and then you wouldn't need the if statement at all, both blocks could be merged.

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