4
\$\begingroup\$

I wrote a little script that asks the user to choose a number between the 0 and 9. Wile he/she is entering a choice, the program tells him/her if the number is too high and too low.

Nothing fancy, just a couple of while and ifs. It's very ugly and has imperative code with mixed responsibilities. I was wondering if there are some Ruby idioms that would make this code more readable and compact. I'm looking for some functions like in functional languages, or maybe there are special Ruby functions I'm not aware off. I don't want to define functions and classes for this simple program.

keepGoing = true
numberToGuess = rand(10)
firstRun = true
response = ''
while keepGoing
    if firstRun
        puts 'Find the magic number between O and 9'
        firstRun = false
    end
    proposedN = gets.chomp.to_i
    if proposedN == numberToGuess
        firstRun = true
        puts "You found the correct number : #{numberToGuess}"
        puts 'Another try ? (Y/N)'
        numberToGuess = rand(10)
        response = gets.chomp
        while response != 'N' and response != 'Y'
            puts 'Please fill in either Y or N'
            response = gets.chomp
        end
        response == 'N' ? keepGoing = false : keepGoing = true
        response = ''
    else
        if proposedN > numberToGuess 
            puts 'Too high'
        else
            puts 'Too low' 
        end
    end
end
\$\endgroup\$
1
\$\begingroup\$

Some ideas:

  1. I wouldn't use while. I would use loops instead, and then exit those loops when needed.
  2. You can use return to get out of the method, and break to exit loops.
  3. I would use methods to break the big chunk of code into several smaller ones.
  4. In Ruby it's common practice to indent with 2 spaces, and for variables to use underscore_names over camelCase.

As a first refactor, I would try to avoid repeating the logic of getting the number:

def game
  number = ask_for_number_first_time
  loop do 
    picked_number = rand(10)
    loop do
      if number == picked_number
         print_win_message(number)
         if another_try? == 'Y'
           break
         else
           return
         end
       else
         print_lose_message(number, picked_number)
       end
     end
   end
 end

def another_try?
  puts 'Another try ? (Y/N)'
  loop do
    response = gets.chomp
    return response if ['N', 'Y'].include?(response)
    puts 'Please fill in either Y or N'
   end
end

def print_lose_message(number, picked_number)
  error = (number > picked_number) ? 'high' : 'low'
  puts "Too #{error}"
end

This would be a first refactor for an exercise.

In general, nested loops are a bad smell in Ruby. In "real-life" you will want to use objects, classes and mixins to divide the responsibility of different parts of the code.

Hope this helps :)

\$\endgroup\$
  • \$\begingroup\$ You ask the number only once and then print "Too high/low" again and again and again and again and again...? \$\endgroup\$ – Nakilon Aug 3 '15 at 3:11
2
\$\begingroup\$

Well, you already identified the main issue with your code, and I concur: the imperative style. Also, note that this code should have two loops and you are doing it all in one, that's why you have to use flags all over the place. Since you asked for a functional solution, no while nor loop, use recursion:

def guess_number(number_to_guess)
  guess = gets.to_i
  if guess == number_to_guess
    puts("You found the correct number: #{number_to_guess}")
  else
    puts(guess > number_to_guess ? "Too high" : "Too low")
    guess_number(number_to_guess)
  end
end

def game
  puts('Find the magic number between O and 9')
  guess_number(rand(10))
  print('Another try? (Y/N): ')
  if gets.strip.downcase == "y"
    game
  end
end

game

If you want to validate the input, create a method def get(validation_regexp) and use it: get(/^\d+$/) and get(/^[yYnN]$/).

\$\endgroup\$
  • 1
    \$\begingroup\$ This is nice--concise yet easy to read. \$\endgroup\$ – Mark Thomas Jul 27 '15 at 17:20
  • \$\begingroup\$ Why strip before your to_i call? An integer will not have anything that would be stripped out anyway. It is very similar to chomp.to_i \$\endgroup\$ – vgoff Aug 2 '15 at 7:51
  • \$\begingroup\$ indeed, updated \$\endgroup\$ – tokland Aug 2 '15 at 19:21

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.