Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a Ruby script meant for some basic database tasks, such as add a user and deactivate a user. The end user can load up the script and follow the basic path of "Do I want to add a user?", or "Do I want to deactivate a user?". They have the option to retreat back to the main menu if needed.

The script works just fine, but being new to Ruby, I am starting to sense I have made things overly complicated and some simple refactoring could make the code much cleaner. I am also interested to hear any other feedback.

  require 'yaml'

  config = YAML.load_file('config.yml')  ## Loads in the config file with databases eligible for selection

  puts 'Loading configured Databases'

  config.each do |k, v|
    puts " #{k}, #{v}"
  end

  def path  # Method to create a reusable menu
    puts "Press 1 to add a new user\nPress 2 to mark a user as active or inactive \nPress 3 to Exit"

    path = gets.chomp.to_i

    if path == 3
      puts 'Exiting...'
      exit
    else
      path_switch(path)
    end
  end

    def path_switch(path_var)
      config = YAML.load_file('config.yml')  ## Loads in the config file with databases eligible for selection
      path = path_var
      rds = ''
      table = ''
      choicelen = config.length
    case path
      when 1
        puts 'Select the numeric ID of the ICE installation to add the user to\n Enter 0 to go back'
        config.each do |k, v|
          puts "#{v['id']}: #{k}"
        end
          choice_client_ice = gets.chomp.to_i
           if choice_client_ice > choicelen
             puts 'error, not a valid selection'
             path
           end
          path if choice_client_ice == 0
          config.each do |k, v|
            if v['id'].to_i == choice_client_ice
              rds = v['rds']
              table = v['table']
            end
          end
            puts "Your current focus is: #{rds} at #{table}"
            path_add(rds, table)
      when 2
        puts 'Select the numeric ID of the ICE installation to edit the user for or type in 0 to go back'
        config.each do |k, v|
          puts "#{v['id']}: #{k}"
        end
          choice_client_ice = gets.chomp().to_i
          path if choice_client_ice == 0
          if choice_client_ice > choicelen
            puts 'error, not a valid selection'
            path
          end
          config.each do |k, v|
            if v['id'].to_i == choice_client_ice
              rds = v['rds']
              table = v['table']
            end
          end
            puts "Your current focus is: #{rds} at #{table}"
            path_edit(rds, table)
        else
          puts "Error, #{path} is not a valid option."
          path
      end
  end


  def path_add(rds, table)
    puts 'Input the users email address or enter the word exit to go back to the beggining'
    email = gets.chomp
    if email == 'exit'
      path
    else
    puts 'Please add the users group id'
    groupid = gets.chomp.to_i
    puts 'Please add the users course style id'
    coursestyleid = gets.chomp.to_i
    puts "This will create a user with email address #{email}, group id of #{groupid} and course style id of #{coursestyleid}.  Please confirm (y/n)"
    confirm = gets.chomp
    if confirm == 'y'
      ## Add in sequel call
      puts 'user has been added as xxx'
    else
      path
    end
  end
  end

  def path_edit(rds, table)
    puts 'Input the users ID or enter the word exit to go back to the beggining'
    uid = gets.chomp
    if uid == 'exit'
      path
    else
      puts 'The user xxx is currently xxx, this will mark him as xxx, continue? (y/n)'
      confirm = gets.chomp
        if confirm == 'y'
          ## Add in sequel call
          puts 'User has been marked as xxx'
        else
          path
        end
    end
  end

  def ice_select
  end

path
share|improve this question

1 Answer 1

up vote 3 down vote accepted

Some high level advice:

  1. Fix your indendation. For example, the indentation is randomly increased on this line: choice_client_ice = gets.chomp.to_i. I mention this only because there is a lot of deep nesting here, which is confusing enough without incorrect indentation. Which brings me to...
  2. Try to rewrite this entire thing, allowing yourself only 1 level of indentation (or 2, at most). This will force you to separate high level logic and implementation detail, by pulling implementation out into its own functions.
  3. Try to rewrite it, keeping your program logic and "views" separate. All the puts statements are "view" code -- they are to your command line app what html files are to web apps. Think about writing functions with names like main_menu_selection, which do nothing but present the user with his choices, get his selection, and return it.
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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