patternrubyrailsMinor
Client categorisation based on hourly payment
Viewed 0 times
categorisationpaymenthourlyclientbased
Problem
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:
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?
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 = 400
return 'B' if hourly_rate >= 350
return 'C' if hourly_rate >= 300
return 'D' if hourly_rate < 300
end
endThis 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?
Solution
I like @SergeiiK's first solution, but we can make this more declarative:
You'd probably want to add some error handling to
Note that the inner Hashes are Hashes for aesthetics only. Using
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)
GRADES[client.country].find do |_, range|
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.Code Snippets
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)
GRADES[client.country].find do |_, range|
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"Context
StackExchange Code Review Q#129434, answer score: 3
Revisions (0)
No revisions yet.