A while ago, I posted about creating a user tool for forum admins that allows them to add, edit, and delete users from their desktops. This still is not complete, posting for review on tool so far

I've finished with the delete_user method and also added a couple new features to it, what this code does, is take your input, and save it to a YAML file ( will be converting YAML to JSON ). It's advantages are that you don't have to go directly to your database until you actually need to uplaod the file, so you can create an entire list of users from your desktop without sending one validation e-mail, or having to validate one user.

#!/usr/bin/env ruby

require 'yaml'

print "Welcome to User Menu. Would you like to [A]dd users, [D]elete users, [E]dit users, [V]iew all users, or [Q]uit the system?\n"
input = gets.chomp!
case input
when /a/i
when /d/i
delete_user
when /e/i
edit_user
when /v/i
view_all
else
puts "Exiting system.."
exit
end
end

File.exist?('users.yml') ? YAML.load_file('users.yml') : {users: {}}
end

def save_hash(hash)
File.open('users.yml', 'w') { |s| s.write(hash.to_yaml) }
end

member_status: prompt("Enter member status:")}
save_hash(hash)
end

def delete_user
if hash[:users].has_key?(to_delete)
hash[:users].delete(to_delete)
else
end
save_hash(hash)
delete_user if restart
end

def edit_user
edit_user if restart
end

def view_all
end

def restart
puts "Would you like to edit another user?"
input = gets.chomp
if input =~ /yes/i
return true
else
puts "Exiting program.."
exit
end
end

def prompt ( message )
puts message
gets.chomp
end


Overview of the YAML file:

---
:member_status: TEST


Is there anything here that I can do better? Better syntax, combine methods, edit something? I'm looking for every possible example.

## Bug

Adding a user with a blank username is allowed. (Maybe you should allow alphanumeric characters only, and set a minimum required length)

## Unbounded recursion

def add_user
# ...
end


The recursion you're using for repeating this function can grow arbitrarily deep, causing a stack overflow error. Use a loop instead:

def add_user
loop do
# ...
break unless restart
end
end


Then, restart doesn't need to harshly exit the whole program. It can simply return a boolean:

def restart
puts "Would you like to edit another user?"
input = gets.chomp
return input =~ /yes/i
end


(You can print "Exiting program" at the end of the program, if you want)

Also, don't exit in menu. It's not needed.

### Nitpicking

print "...?\n"


Use puts instead of using a newline in the string.

if hash[:users].has_key?(to_delete)
hash[:users].delete(to_delete)
else
end


Hash#delete returns the deleted value, or nil if the key is not found. So you can use its return value for the condition:

user = hash[:users].delete(to_delete)
unless user
end


Also, rename to_delete to username.

puts "Would you like to edit another user?"


This message appears for every action, not just editing. Pass a string argument to specify the action (e.g. "add" or "edit") and use it when printing the message. Also, rename restart to again? to be more accurate.

## Creating a class

Most methods operate on the same data structure: the users hash. So it makes sense to put them all inside a class which stores the hash as a field. As a bonus, you won't need to reread the users file every time you perform an action.

class UsersList
member_status: member_status
}
end

end

def save
save_hash
end

def to_s # for viewing
file.to_s
end

private

def file
# Return @hash, which stores the hash
# If it's not loaded yet (@hash is nil), initialize it with load_hash first
end

File.exist?('users.yml') ? YAML.load_file('users.yml') : {users: {}}
end

def save_hash
File.open('users.yml', 'w') { |s| s.write(@hash.to_yaml) }
end
end


Notice all methods for accessing the hash use the private method file. (I named it file because hash is an existing method of all objects). file returns the hash, but only loads it once from the file, when it's first needed.

Note the methods only deal with the logic of the operations, not with interacting with the user, in order to seperate these two concerns.

To use this class, you'll need to make an instance of it:

$users_list = UsersList.new # global variable for handling the users list  Then change add_user, delete_user and view_all to use it: def add_user loop do username = prompt('Enter username: ') email_address = prompt('Enter email:') member_status = prompt('Enter member status:')$users_list.add(username, email_address, member_status)
unless $users_list.remove(username) puts 'User not found' end break unless again?('delete') end end def view_all puts$users_list.to_s

$users_list.save  • In order to make it so the username can only contain certain elements and a required length would I have to run it against another method? Nov 15 '15 at 18:19 • @LostBam In UsersList#add you can check the username against a regex (e.g. [/\A[a-zA-Z0-9]{6,}\z/] rubular.com/r/mHcUaXbjMA)), and return false if it doesn't match. Then in add_user check the return value (like delete_user does with $users_list.remove) Nov 15 '15 at 18:27