CODEWITHSUNDEEP
ruby-on-railsruby
Alex
Alex

Reputation: 65

Shortening a long if else statement

I have a long if else statement:

 rnd = rand(1..1000)
 if rnd >= 600
   0
 elsif rnd < 600 && rnd >= 350
   1
 elsif rnd < 350 && rnd >= 270
   2
 elsif rnd < 270 && rnd >= 200
   3
 elsif rnd < 200 && rnd >= 150
   4
 elsif rnd < 150 && rnd >= 100
   5
 elsif rnd < 100 && rnd >= 80
   6
 elsif rnd < 80 && rnd >= 50
   7
 elsif rnd < 50 && rnd >= 30
   8
 else
   9
 end

I would like to shorten it. Is it possible? My rubocop swears at this long method.

Upvotes: 1

Views: 127

Answers (4)

Cary Swoveland
Cary Swoveland

Reputation: 110675

MX = 1000
LIMITS = [600, 350, 270, 200, 150, 100, 80, 50, 30, 0]

The required index can be computed as follows.

def doit
  rnd = rand(1..MX)
  LIMITS.index { |n| n <= rnd }
end

doit
  #=> 5

In this example rnd #=> 117.

If this must be repeated many times, and speed is paramount, you could do the following.

LOOK_UP = (1..MX).each_with_object({}) do |m,h|
  h[m] = LIMITS.index { |n| n <= m }
end
  #=> {1=>9, 2=>9,..., 29=>9,
  #    30=>8, 31=>8,..., 49=>8,
  #    ...
  #    600=>0, 601=>0,..., 1000=>0}

Then simply

def doit
  LOOK_UP[rand(1..MX)]
end

doit
  #=> 3

In this example rand(1..MX) #=> 262.

If speed were paramount but MX were so large that the previous approach would require excessive memory, you could use a binary search.

def doit
   rnd = rand(1..MX)
   LIMITS.bsearch_index { |n| n <= rnd }
end

doit
  #=> 5

In this example rnd #=> 174.

See Array#bsearch_index. bsearch_index returns the correct index in O(log n), n being LIMITS.size). bsearch_index requires the array on which it operates to be ordered.

Upvotes: 2

Mark G.
Mark G.

Reputation: 3260

Great answers already! Just chiming in since I had a suspicion that ruby could handle this with a case statement, and it appears to be able to do so:

rnd = rand(1..1000)

case rnd
when 600.. then 0
when 350...600 then 1
when 270...350 then 2
...
else 9
end

Regardless of the approach taken, you're going to have to specify the ranges somewhere, so I think using something like a case statement is appropriate here (sorry! It doesn't shorten the code more than a few lines). Using a hash would also be a great approach (and might allow you to move the hash elsewhere), as other commenters have already shown.

It's worth mentioning, with ruby ranges, .. means that the range is inclusive and includes the last value (1..10 includes the number 10), and ... means the range is exclusive where it does not include the last value.

The top case 600.. is an endless range, which means it will match anything greater than 600. (That functionality was added in ruby 2.6)

Upvotes: 5

babgyy
babgyy

Reputation: 343

You can simplify your conditions by using only the lower bound. And you can avoid repeting elsif because it is cumbersome

rnd = rand(1..1000)
lower_bounds = {
  600 => 0,
  350 => 1,
  270 => 2,
  200 => 3,
  150 => 4,
  100 => 5,
  80 => 6,
  50 => 7,
  30 => 8,
  0 => 9,
}

lower_bounds.find { |k, _| k <= rnd }.last

Upvotes: 2

spickermann
spickermann

Reputation: 106802

I would start with something like this:

RANGES = {
  (0...30) => 9,
  (30...50) => 8,
  (50...80) => 7,
  # ...
  (350...600) => 1,
  (600...1000) => 0
}

rnd = rand(1..1000)
RANGES.find { |k, _| k.cover?(rnd) }.last

Upvotes: 7

Related Questions