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 - Salome & Kee - Word Guess #24

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

Conversation

swubeshet
Copy link

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 both tried hard to make contributions and shared work equally.
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 method .guess_letter helped us identify if the user input was inside the word. It took guess as the input and showed art, the letters guessed and the current status of the word the user was guessing.
Describe an instance where you used a local variable instead of an instance variable. Why did you make that choice? We used the local variable 'select_art' because we needed to save that data to print out at that moment, and didn't need it anywhere else in the code.
What code, if any, did you feel like you were duplicating more than necessary? In the beginning, in our initialize method we had variables for each step of converting the answer into the answer with blanks
Is there a specific piece of code you'd like feedback on? The initialize method and the guess_letter method. Overall, it would be nice to know what parts could be DRY-er.

@droberts-sea
Copy link

Word-Guess Game

What We're Looking For

Feature Feedback
Baseline
Regular Commits with meaningful commit messages. some - In the future, I would like to see more descriptive commit messages. Something like "update word_guess.rb and game.rb" doesn't tell the reader much about how or why the code changed. Instead, go for something like "Added logic to handle ascii art" or "Fixed a bug where the user would always win".
Readable code with consistent indentation. no - Your indentation is all over the place in game.rb, and in word_guess.rb you use tabs instead of spaces.
Answered comprehension questions yes
Product Functionalities
Created a Class to encapsulate game functionality. yes
Used methods to DRY up your code. yes - You asked about opportunities to DRY up your code in the comprehension questions, but I can't see any major repetition.
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. yes - good work!

Great work overall! I've got a few comments below, but overall I'm pretty happy with what you've turned in. Just make sure to watch your indentation better in the future.

@hidden_answer = @one_answer.gsub(/[a-z]/, '_')
@art =[
" _
\/\\ )\\

Choose a reason for hiding this comment

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

This art takes up a large amount of space in your file, making it difficult to read. A good solution to this is to put it into another file, something like art.rb.

Choose a reason for hiding this comment

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

That will also let you use Atom's auto-indent functionality without messing up your art.


def guess_letter(guess)
guess_result = @answer.index(guess)
if guess_result == nil # negative result

Choose a reason for hiding this comment

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

Watch indentation here.

require 'Faker'
class Game

def initialize

Choose a reason for hiding this comment

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

Instance methods should be indented within their class.

@answer = Faker::Dessert.variety.downcase
no_spaces = @answer.delete(" ")
no_split = no_spaces.split("")
@one_answer = no_split.join(" ")

Choose a reason for hiding this comment

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

I like your word selection!

puts answer_array
matching_indices = answer_array.each_index.select {|i| answer_array[i] == guess }
matching_indices.each do |i|
replacement_letter = i

Choose a reason for hiding this comment

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

Good use of enumerables to solve this problem.


def red
colorize(31)
end

Choose a reason for hiding this comment

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

Where are these methods used?

puts "Start guessing!\n\n#{@art[0]}\n\n#{@hidden_answer}"
end

def guess_letter(guess)

Choose a reason for hiding this comment

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

I like that handling a guess is its own method, but it ends up being pretty complicated. Would it be possible to split it further into multiple small methods? Might make it easier to work with.

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