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
3 Answers 3
It looks good and is readable.
A few notes :
current_state = "#{@display_content}"
could be writtencurrent_state = @display_content.clone
words[rand(words.length)].strip
could bewords.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:
- when the game has been successfully
_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!
-
\$\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– JMac2018年03月21日 23:35:13 +00:00Commented 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\$JMac– JMac2018年03月21日 23:44:59 +00:00Commented 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\$Eric Duminil– Eric Duminil2018年03月22日 11:19:49 +00:00Commented Mar 22, 2018 at 11:19
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
:-)
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 ""
).
Explore related questions
See similar questions with these tags.