2
\$\begingroup\$

I just finished this command-line implementation of the Mastermind board game that allows two players to play against each other. I'm fairly new to Ruby, but I think I did a decent job with this. However, I'm sure there are some things I could've done better, so please let me know if there are.

class Codebreaker
 attr_accessor :colors
 def initialize
 @colors = []
 end
end
require 'io/console'
class Codemaker
 attr_accessor :code_colors
 def make_code
 puts "\nCodemaker, select 4 colors! Please enter them separated by spaces, with no commas. To prevent cheating, your input will be hidden. Try to not make any typos :)"
 input = STDIN.noecho(&:gets).chomp
 @code_colors = input.split(" ")
 puts "Excellent!"
 end
end
class Game
 require_relative "codebreaker.rb"
 require_relative "codemaker.rb"
 def initialize
 @code_breaker = Codebreaker.new
 @code_maker = Codemaker.new
 @turn = 1
 start_game
 end
 def start_game
 puts "Welcome to Mastermind!
 \nRules: 
 \nThe codemaker creates a code consisting of four colors chosen from the provided list. 
 It is the goal of the codebreaker to crack the code in 12 turns, entering the same exact colors in the same order. To attempt to crack the code, simply
 keep guessing. 
 \nYou may choose from the following colors: red, blue, green, yellow, orange, purple, white, and black."
 @code_maker.make_code
 turn
 end
 def turn
 while @turn < 12
 code_breaker_input
 compare
 @turn += 1
 if @turn == 12
 puts "Unfortunately, you didn't guess the codemaker's code in the allotted 12 turns. Better luck next time!"
 exit
 end
 end
 end
 def code_breaker_input
 puts "\nCodebreaker, please make your selection! Enter colors in lowercase, separated by spaces, with no commas."
 @code_breaker.colors = gets.chomp
 end
 def compare
 b_colors = @code_breaker.colors.downcase.split(" ")
 m_colors = @code_maker.code_colors
 correct_indexes = []
 correct_colors = []
 b_colors.zip(m_colors).map { |a, b|
 if a == b
 correct_indexes << "X"
 else correct_indexes << "O" end
 }
 b_colors.each { |color|
 m_colors.any? { |c_color|
 correct_colors << color if c_color == color
 }
 }
 puts correct_indexes.join(" ")
 puts "Colors guessed correctly: #{correct_colors.join(", ")}"
 #checks win condition
 if correct_indexes.all? { |index| index == "X" }
 puts "The codemaker cracked the code!"
 exit
 end
 end
end
game = Game.new
```
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 4, 2019 at 22:28
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Simple things you can improve:

  • 12 is a magic number in your code, you can move it to a constant.
  • I think TURNS.times do |turn| ... is better than the while loop because you don't have to keep track of the counter.
  • When you're passing a simple space (" ") to the method split you can just call split instead, without passing any argument.
  • When the block has multiple lines it is a good practice to use do and end instead of brackets.
  • I think you can use the select method and assign the result directly to the correct_colors variable.
  • I think it would be better to call Game.new.start_game instead of Game.new, for me, it isn't intuitive that the game will begin just by instantiating the Game class.
  • You can make some methods private because you are not calling them on the instance.
answered Nov 21, 2019 at 0:43
\$\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.