5
\$\begingroup\$

I'm just a few months in learning to code and never had my code reviewed. Is this class structure ok? Should I split it differently? How is the code overall?

require "json"
class Hangman
 def initialize
 @secret_word = select_word
 @display_content = "_" * @secret_word.length
 @failed_attemps = 0
 end
 def main_menu
 option = "3"
 until option == "1" || option == "2"
 puts "(1) New game"
 puts "(2) Load game"
 print "Play new game or load the saved game? "
 option = gets.chomp[0]
 if option == "2" 
 if File.exist?("saved_state.json")
 load_state
 else
 puts "There is no saved game, save one first"
 option = "3"
 end
 end
 end
 start_game
 end
 private
 def save_state
 json_object = { 
 :secret_word => @secret_word, 
 :display_content => @display_content,
 :failed_attemps => @failed_attemps 
 }.to_json
 File.open("saved_state.json", "w") { |file| file.write(json_object) }
 end
 def load_state
 save_file = File.read("saved_state.json")
 json_hash = JSON.parse(save_file)
 @secret_word = json_hash["secret_word"]
 @display_content = json_hash["display_content"]
 @failed_attemps = json_hash["failed_attemps"]
 end
 def start_game
 player_won = false
 while @failed_attemps != 10 
 puts @display_content
 puts "#{10 - @failed_attemps.to_i} turns left" 
 print "Enter a letter or attempt the full word: "
 letters = gets.chomp
 if letters == "save"
 save_state
 next
 end
 break if letters == "exit"
 update_display(letters) if letters
 player_won = player_won?
 break if player_won
 end
 puts "Game over, the secret word was: #{@secret_word}" if @failed_attemps == 10
 end
 def select_word
 words = File.readlines("5desk.txt").select { |word| word.length.between?(5, 12) }
 words[rand(words.length)].strip
 end
 def update_display(letters)
 letters.downcase!
 current_state = "#{@display_content}"
 if letters.length == 1
 @display_content.length.times do |index|
 @display_content[index] = letters if @secret_word[index].downcase == letters
 end
 else
 @display_content = letters if letters == @secret_word.downcase
 end
 current_state == @display_content ? print_toon(1) : print_toon(0)
 end
 def player_won?
 unless @display_content.include?("_")
 puts "You found the correct word!"
 true
 end
 end
 def print_toon(increment)
 @failed_attemps += increment
 case @failed_attemps
 when 0
 puts " ______"
 puts " |"
 puts " |"
 puts " |"
 puts " |"
 when 1
 puts " ______"
 puts " | |"
 puts " |"
 puts " |"
 puts " |"
 when 2
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts " |"
 puts " |"
 when 3
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts " | |"
 puts " |"
 when 4
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts " || |"
 puts " |" 
 when 5
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts "/|| |"
 puts " |"
 when 6
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts "/||\\ |"
 puts " |"
 when 7
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts "/||\\ |"
 puts "/ |"
 when 8
 puts " ______"
 puts " | |"
 puts "(oo) |"
 puts "/||\\ |"
 puts "/ \\ |"
 when 9
 puts " ______"
 puts " | |"
 puts "(ox) |"
 puts "/||\\ |"
 puts "/ \\ |" 
 when 10
 puts " ______"
 puts " | |"
 puts "(xx) |"
 puts "/||\\ |"
 puts "/ \\ |" 
 end
 puts ""
 end
end
my_game = Hangman.new
my_game.main_menu
asked Mar 15, 2018 at 16:16
\$\endgroup\$
0

3 Answers 3

1
\$\begingroup\$

It looks good and is readable.

A few notes :

  • current_state = "#{@display_content}" could be written current_state = @display_content.clone
  • words[rand(words.length)].strip could be words.sample.strip
  • select { |word| word.length.between?(5, 12) } will select words with a length between 4 and 11 due to the newline.
  • It would be nice to have some more feedback:
    • when the game has been successfully saved
    • when the game has been loaded, what the drawing looks like
    • when the last letter has been guessed, what the complete word was. Here's an example output:

_u_ar_
4 turns left
Enter a letter or attempt the full word: s
 ______
 | |
(oo) |
/||\ |
 |
su_ars
4 turns left
Enter a letter or attempt the full word: g
 ______
 | |
(oo) |
/||\ |
 |
You found the correct word!
  • You could replace the whole case;when statement with an array of strings.

Keep up the good work!

answered Mar 21, 2018 at 12:50
\$\endgroup\$
3
  • \$\begingroup\$ Awesome feedback! Thanks. The game actually ends in victory if you input the correct word, in the method update_display the "_____" becomes the correct word and player_won? method sees there is no "_" so triggers a victory. Maybe I should rewrite that part to make it more straight forward. I just tested and it works for me without changing the variables as you said, if I enter directly the correct word at any point the game ends. Can you tell me how to reproduce the bug? If you mean when you guess just one letter correctly, it does show the actual state of the hangman but doesn't "increase it" \$\endgroup\$ Commented Mar 21, 2018 at 23:35
  • \$\begingroup\$ About the current_state = @display_content.to_s its inside a #{} operator so it does a expression substitution taking only the string value. \$\endgroup\$ Commented Mar 21, 2018 at 23:44
  • \$\begingroup\$ @JMac: Sorry about that, it seems rubocop was overzealous and introduced a bug. I'll check it again. \$\endgroup\$ Commented Mar 22, 2018 at 11:19
1
\$\begingroup\$

A very minor style detail is the hash. Where you have:

json_object = { :secret_word => @secret_word, :display_content => @display_content,
 :failed_attemps => @failed_attemps }.to_json

I would amend the indentation, so that each line of the code is doing one distinct thing. I would also use the Ruby 1.9 hash syntax:

json_object = {
 secret_word: @secret_word,
 display_content: @display_content,
 failed_attemps: @failed_attemps
}.to_json

Also, I would change the name of the variable failed_attemps to failed_attempts :-)

answered Apr 15, 2018 at 1:01
\$\endgroup\$
0
\$\begingroup\$

The first thing I noticed was the escaped \\; you don't need that if you use 'hello\world' instead of "hello\\world" (In general with literal strings it's better to use '' because you don't have to pay as much attention to escapes as with "").

answered Apr 5, 2018 at 9:13
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.