# Client categorisation based on hourly payment

I am currently working on a small internal rating system for clients to be able to prioritise my work based on what client pays best. The input data is the hourly rate they are paying and based on that they should be divided into categories in are the range from A-D. The first setup looks like this:

def category_for(client)
hourly_rate = client.hourly_rate
case client.country
when :germany
return 'A' if hourly_rate >= 35
return 'B' if hourly_rate >= 30
return 'C' if hourly_rate >= 25
return 'D' if hourly_rate < 25
when :sweden
return 'A' if hourly_rate >= 400
return 'B' if hourly_rate >= 350
return 'C' if hourly_rate >= 300
return 'D' if hourly_rate < 300
end
end


This is ofc only part of it to show the general setup.

This code, although working fine, really really hurts my eyes, so I was wondering if someone here had a good idea on how to refactor it in a nice way?

I like @SergeiiK's first solution, but we can make this more declarative:

GRADES = {
germany: {
"A" => 35...Float::INFINITY,
"B" => 30...35,
"C" => 25...30,
"D" => 0...25,
},
sweden: {
"A" => 400...Float::INFINITY,
"B" => 350...400,
"C" => 300...350,
"D" => 0...300,
},
}

def category_for(client)
range.cover?(client.hourly_rate)
end.first
end

Client = Struct.new(:country, :hourly_rate) # just for demonstration

p category_for(Client.new(:germany, 26))
# => "C"

p category_for(Client.new(:sweden, 7000))
# => "A"


You'd probably want to add some error handling to category_for for invalid inputs.

Note that the inner Hashes are Hashes for aesthetics only. Using Enumerable#find on a Hash will iterate over its key-value pairs one by one just as it would an array of key-value pairs ([ [ "A", 35...Float::INFINITY ], ... ]), making lookup O(n) instead of O(1) (ish) like normal Hash lookup. This is just prettier than nested arrays. This won't be as fast as a case solution, either, but unless you're, say, "grading" hundreds of thousands of clients at once you won't notice a difference.

• I really like your approach, thx for sharing! – Severin May 30 '16 at 7:26

The pattern return something if condition is ok as a guard to check preconditions, but not to write the core logic of the method (IMHO), which should use expressions preferably. I think a simple case it's much more clear:

def category_for(client)
hourly_rate = client.hourly_rate
case client.country
when :germany
case
when hourly_rate >= 35 then 'A'
when hourly_rate >= 30 then 'B'
when hourly_rate >= 25 then 'C'
when hourly_rate < 25 then 'D'
end
when :sweden
case
when hourly_rate >= 400 then 'A'
when hourly_rate >= 350 then 'B'
when hourly_rate >= 300 then 'C'
when hourly_rate < 300 then 'D'
end
end
end


Now, if this started to grow, or the intervals had to be configurable, I'd extract those values to data structures. Something like this, you get the idea:

CATEGORIES_BY_HOURLY_RATE = {
germany: [35, 30, 25],
sweden: [400, 350, 300],
}


I suggest further improvement (using ranges):

def category_for(client)
hourly_rate = client.hourly_rate
case client.country
when :germany
case hourly_rate
when 0...25  then 'D'
when 25...30 then 'C'
when 30...35 then 'B'
else              'A'
end
when :sweden
case hourly_rate
when 0...300   then 'D'
when 300...350 then 'C'
when 350...400 then 'B'
else                'A'
end
end
end


And finally, I come with this DRY solution (to avoid nested case duplication):

def category_for(client)
hourly_rate = client.hourly_rate
# common_quotient acts like cross-course
common_quotient = client.country == :germany ? 1 : 11.5
common_hourly = hourly_rate / common_quotient

case common_hourly
when 0...25  then 'D'
when 25...30 then 'C'
when 30...35 then 'B'
else              'A'
end
end

• I like the ranges. I can't say that I like the currency conversion formula, though. – 200_success May 27 '16 at 8:23
• Me neither, but it's really not good idea to nest multiple case statements. If you'll add another currency quotient, it will just affect one if/case statement. – Sergii K May 27 '16 at 8:28
• The problem is, the thresholds aren't proportional, and your conversion isn't exact, so you could get the wrong answer for non-integer rates. – 200_success May 27 '16 at 8:34
• Thx for your suggestion. I quite like the general approach, but I agree with @200_success on the conversion formula. Especially if I wanna add more currencies this creates an additional layer of complexity. – Severin May 27 '16 at 11:26
• If you pay for work you've done - that is really something goes wrong, don't you think? – Sergii K May 28 '16 at 4:58