def route_action(action)
    case action
    when 1 then @meals_controller.add
    when 2 then @meals_controller.list
    when 3 then @meals_controller.edit
    when 4 then @meals_controller.delete
    when 5 then @customers_controller.add
    when 6 then @customers_controller.list
    when 7 then @customers_controller.edit
    when 8 then @customers_controller.delete
    when 0 then stop
    else
      puts "Please press 1, 2, 3, 4, 5, 6, 7, 8 or 0"
    end
  end

So I want to reduce this case when is there another way of implementing this?

CodePudding user response：

Heads up, this is just a general suggestion on how to use a bit of Ruby meta-programming to get out of the tangle of having to declare a long method full of case business logic. It won't fit perfectly and will need to extra work to do, for instance, the "quit" logic.

Also. I'll reiterate what's been said in one of the direct answers to the post. Your case solution is VERY clear. It's good code and we shouldn't jump into messier thing just to comply to the all-mighty gods of the ruby stylesheet guidelines. Just because a method is under 10 lines, it doesn't make it automatically better than an 11 (or... hell 40) lines one.

Now...

Here's a meta-programming suggestion...

You can define a hash on a constant to hold the variables needed for the business logic:

ROUTES = [
  { action: :add,  controller: :meals, description: "Add a meal" },
  { action: :list, controller: :meals, description: "List all meals" },
  { action: :add,  controller: :customers, description: "Add a customers" },
  { action: :list, controller: :customers, description: "List all customers" },
]

You can then create a method that dispatches the user to the correct controller action using the hash info:

def dispatch(action_index)
  route_action     = ROUTES[action_index][:action]
  route_controller = ROUTES[action_index][:controller]
  instance_variable_get("@#{route_controller}_controller").send(route_action)
end

It's very easy with the hash to iterate over it to display the route descriptions:

def display_options
  ROUTES.each_with_index do |route, index|
    puts "#{index   1}. #{route[:description]}"
  end
end

You can at some point in your code dispatch the user to the relevant controller action with dispatch(gets.chomp.to_i - 1).

Cool thing about the hash is that you can always add more routes to it by just adding a single line.

CodePudding user response：

One possibility would be to have an array of message names to send based on the action input which is used as an index, and by determining which controller to send to by dividing by 5. Inputs from 1 to 4 will yield 0, while 5 to 8 will yield 1.

def route_action
  actions = [:add, :list, :edit, :delete]

  case action
    when 1..8 
      controller = action / 5 < 1 ? @meals_controller : @customers_controller
      msg = actions[(action - 1) % 4]
      controller.send(msg)
    when 0 
      stop
    else
      puts "Please press 1, 2, 3, 4, 5, 6, 7, 8 or 0"
  end
end

CodePudding user response：

While I have no objection to the code you posted as it is very understandable, we could modify this to the following as well (I took some liberties with the missing context, such as stop and the defined instance variables)

module MyActionRouter
  extend self
  CONTROLLERS = [:meals_controller ,:customers_controller]
  MECHANISMS = [:add, :list, :edit, :delete]
  ACTIONS = CONTROLLERS.product(MECHANISMS).each.with_index(1).to_h.invert
  
  @meals_controller = MealsController
  @customers_controller = CustomersController

  attr_reader *CONTROLLERS

  def route_action(action)
    return puts("Please enter a number between 1 and #{ACTIONS.keys.max} or press 0 to stop") unless (0..ACTIONS.keys.max).cover?(action)
    controller,action = ACTIONS.fetch(action, :stop)
    public_send(controller,action)
  end

  def stop(*)
    'STOP'
  end
  def print_options 
    puts "Select an Option:" 
    puts "0: to stop the process"
    ACTIONS.each do |k,(controller,mechanism)| 
      puts "#{k}: #{controller} => #{mechanism}" 
    end   
  end 
end

This will make the functionality more extendable in the future

CodePudding user response：

How about:

CASES = [@meals_controller, @customers_controller].product([:add, :list, :edit, :delete])

def route_action(action)
  if action > 0
    selected = CASES[action-1] # something like [:@meals_controller, :add]
    if selected
      controller.send(*selected)
    else
      puts "action number too large"
    end
  elsif action == 0
    stop
  else # Do we have to catch this?
    puts "action number negative" # programming error?
  end
end