2
\$\begingroup\$

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
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 2, 2015 at 22:26
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

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.
answered Jun 15, 2015 at 2:05
\$\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.