CODEWITHSUNDEEP
ruby-on-railsruby
Postscripter
Postscripter

Reputation: 521

Is using a case statement in my ruby on rails example ok?

I have a built a little online tool using rails. The app has no database. (I know I should have used another simpler framework, like Sinatra for Ruby). So I need to capture selected car model from the interface and use its power, speed, and dimensions properties Is it ok to use case statement, like the code below:

class Car
def get_car_properties (dropdown_selection)
    info = {}
    info=Hash.new{2}
    case dropdown_selection
        when 'Mazda 3'
             info = { speed: 'fast', power: 'real strong', dimensions: '4X3X3'}
        when 'Lancer 81'
             info = { speed: 'slow', power: 'real weak', dimensions: '2X2X2'}
         else
             info={}

    end
    return info
end
end

selected_car=Car.new()
puts selected_car.get_car_properties('Mazda 3')

For simplifications, I have not used the exact scenario. Also note that my case statement is about 30 options big and the hash 10 symbols.

Upvotes: 0

Views: 101

Answers (2)

Aniket Schneider
Aniket Schneider

Reputation: 944

Big case statements are a code smell, meaning they are a hint that there may be an opportunity to improve your code.

In this case, while your app does not have a database, you still have data. The reason why your code looks messy right now is that you have not separated that data from your program logic.

I would recommend storing the data about each car in a hash, and then treat that hash as if it were your database.

class Car
  CAR_INFO = {
    'Mazda 3' => {
      speed: 'fast',
      power: 'real strong',
      dimensions: '4X3X3'
    },
    'Lancer 81' => {
      speed: 'slow',
      power: 'real weak',
      dimensions: '2X2X2'
    },
    # etc
  }

  def get_car_properties (dropdown_selection)
    CAR_INFO.fetch(dropdown_selection, {})
  end
end

selected_car=Car.new()
puts selected_car.get_car_properties('Mazda 3')

Why is this better? For one thing, the hash is pure data. It's clear that there's no additional control logic hiding somewhere in one of the branches of your case statement. You can now tell at a glance exactly what get_car_properties is doing - it's looking up the selection in a data structure, and returning an empty hash if nothing is found. You may wish to move CAR_INFO into a separate file, or perhaps into a proper database later on - get_car_properties won't change much in either of those cases.

Upvotes: 2

Daniel Westendorf
Daniel Westendorf

Reputation: 3465

The code you're written probably works, but definitely smells and would violate the ruby style guide.

I suggest you utilize a code analyzer, such as rubocop which will enforce the ruby style guide, and help you write conforming code and avoid common pitfalls.

Upvotes: 0

Related Questions