4
\$\begingroup\$

I've created a program that will take, a bunch of numbers from a range of 1 - 255, and then put them into 3 different strings(servers) equally. It does this by having a count that starts at 1 and loops to 3, once the count has hit 3 it restarts until three servers have hit a max capacity of 100 numbers within it.

Once that max capacity has been hit, the numbers dump the remaining into a server called failsafe.

I'm looking for better ways to write this, that are cleaner, and will make this, if possible, more readable.

#!/local/usr/bin/ruby
users = (1..255) #<= Amount of users
calculon = " " #<= Random servers to be append into
darth = " "
invader_zim = " " 
failsafe = " " #<= Failsafe
count = 1
users.each do |i|
 target_string = case count
 when 1 then calculon #<= Append into server
 when 2 then darth #<= Append into server
 when 3 then invader_zim #<= Append into server
 end
 target_string = failsafe if target_string.length == 100
 target_string << i #<= When amount is reached hits fail safe
 if count == 3
 count = 1
 else
 count += 1
 end
end
puts calculon.length == 1 ? 'No users on server' : calculon.length
puts darth.length == 1 ? 'No users on server' : darth.length
puts invader_zim.length == 1 ? 'No users on server' : invader_zim.length
puts failsafe.length == 1 ? 'No users on server' : failsafe.length

Example of usage:

86
86
86
No users on server

The main objective of this program is to have a script that will equally load three different servers, and if all servers have hit a max capacity, will load into a fail safe server.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Dec 30, 2015 at 18:33
\$\endgroup\$
10
  • \$\begingroup\$ These are weird strings that you are composing. This feels like an XY Problem. Could you tell us more about what you really want to do? \$\endgroup\$ Commented Dec 30, 2015 at 20:40
  • \$\begingroup\$ @200_success Yessir, give me 1 second \$\endgroup\$ Commented Dec 30, 2015 at 20:40
  • \$\begingroup\$ @200_success How's that..? \$\endgroup\$ Commented Dec 30, 2015 at 20:46
  • 1
    \$\begingroup\$ Why do the strings start with a space? Why binary ASCII strings rather than an array of integers? Is the string length just a counting device, or will you be parsing out the results to identify which users got assigned there? \$\endgroup\$ Commented Dec 30, 2015 at 20:57
  • 1
    \$\begingroup\$ Can't you just divide the number of users by 3? This feels a bit too hypothetical to me \$\endgroup\$ Commented Jan 1, 2016 at 19:10

2 Answers 2

4
+50
\$\begingroup\$

This is perfect use case for group_by, which basically does all the work for you in a single line:

We also get the added benefit of having the user ids as arrays instead of strings. This makes more sense, and if for some reason you do want to get the string representation back, you can simply call join(' ') on the array of user ids.

Finally, if you add more servers to your cluster, you just need to edit the single line defining the server names at the top of the file, and everything will still work.

users = (1..255) #<= Amount of users
servers = %w(calculon darth invader_zim)
server_capacity = 100
assigned_users = users.group_by.with_index do |u, i| 
 all_servers_full = (i >= servers.size * server_capacity)
 all_servers_full ? 'failsafe' : servers[i % servers.size]
end
assigned_users.each do |server, users|
 puts "#{server} server has #{users.size} users"
end
answered Jan 1, 2016 at 21:13
\$\endgroup\$
0
1
\$\begingroup\$

In this code-snippet you've 1xcase and 2xif/else statements. It's better to use ruby iterators (like with_index, with_object). This way you're avoiding unnecessary variables and their "manual" incrementing (count in your case). Also try to avoid unexplained "hard-coded" integers(3, 100). Basically they're variables or constants.

Next weird strings... it's hard to understand their purpose. Why use " ", not empty string? Everywhere in code string length is used, not content.

Output lines can be easily organized with each and be more informative.

My variant of your code refactoring:

users = (1..255)
servers = Array.new(3, 0)
failsafe = 0
max_load = 100
users.each.with_index do |i, index|
 current = index%servers.count
 if servers[current] < max_load
 then servers[current] += 1
 else failsafe += 1
 end
end
server_list = servers + [failsafe]
%w(calculon darth invader_zim failsafe).each.with_index do |name, index|
 p "On '#{name}' server currently #{server_list[index] == 0 ? 'NO' : server_list[index]} users."
end
answered Jan 1, 2016 at 20:25
\$\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.