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
\$\endgroup\$
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!

\$\endgroup\$
  • \$\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\$ – JMac Mar 21 '18 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\$ – JMac Mar 21 '18 at 23:44
  • \$\begingroup\$ @JMac: Sorry about that, it seems rubocop was overzealous and introduced a bug. I'll check it again. \$\endgroup\$ – Eric Duminil Mar 22 '18 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 :-)

\$\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 "").

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.