2

I'm using below class with processQuestion function to call other methods.

This function is called by calling CONSTANTS of other classes.

# Is responsible for executing a particular question. Question types are in the Question object. A question will 
# always have a responding method in this class. That method will take the parameters defined by the question and
# should provide the answer in the format expected.
class QuestionProcessor
  NO_ROUTE = "NO SUCH ROUTE"

  def initialize(routeList)
    @routeList = routeList
  end


  # Finds the method and runs it. This should provide the answer object
  def processQuestion(question)
    return eval("get"+question.command+"(question)")
  end


  # Finds the total distance using the exact stations specified, or returns NO_ROUTE if no route was stored in the route list
  # this method ignores the constraints and actions
  def getDistance(question)
    distance = 0
    currentStation = nil

    question.parameters.each do |nextStation|
      if (! currentStation.nil?)
        route = @routeList.getDirectRoute(currentStation, nextStation)
        if (route.nil?)
          return NO_ROUTE
        end
        distance += route.distance
      end
      currentStation = nextStation;
    end

    return distance;
  end


  # Finds the shortest route possible for the given constraint. This method requires a constraint and action to be provided
  def getShortestRoute(question)
    startStation = question.parameters[0]
    endStation = question.parameters[1]

    routeProcessor = ShortestRouteProcessor.new(@routeList, question.constraint, question.action)
    routeProcessor.getRoute(startStation, endStation)

    return routeProcessor.shortestRoute == Constants::INTEGER_MAX ? NO_ROUTE : routeProcessor.shortestRoute
  end


  # Counts the number of routes based on the condition provided. Intended to count the number of routes, but could potentially provide a total distance
  # or anything else produced by the action.
  def getCountRoutes(question)
    startStation = question.parameters[0]
    endStation = question.parameters[1]

    routeProcessor = RouteProcessor.new(@routeList, question.constraint, question.action)
    routeProcessor.getRoute(startStation, endStation)

    return routeProcessor.totalSuccessfulRoutes 
  end
end

I thought this is a good approach to remain DRY but I hear eval is evil.

Is this good approach or should I look for other ways in a more object oriented way?

Passionate Engineer
  • 10,034
  • 26
  • 96
  • 168
  • See http://stackoverflow.com/questions/1902744/when-is-eval-in-ruby-justified & http://stackoverflow.com/questions/637421/is-eval-supposed-to-be-nasty – Agis Feb 28 '14 at 12:32

2 Answers2

3

In this case you may safely use send instead of eval, like in this example:

def processQuestion(question)
  return send("get#{question.command}", question)
end

Just be aware that send may be as dangerous as eval if you do not sanitize your input (question.command in this case).

If possible, do a white-list filtering before calling send (or eval), otherwise someone could pass a command which does something you do not want to do.

Arsen7
  • 12,522
  • 2
  • 43
  • 60
  • what is the difference between send and eval? – Passionate Engineer Feb 28 '14 at 12:45
  • @PassionateDeveloper Eval evaluates anything. Send sends a message. – Dave Newton Feb 28 '14 at 12:45
  • so in a way send is safer as it is more constrained? – Passionate Engineer Feb 28 '14 at 12:48
  • 1
    That's right. Explaining with other words, the `eval` evaluates _any_ ruby code, while `send` calls just some existing method of the object. The parameter you pass to `send` is the method name to be called. Just as a note - the rest of the parameters that you pass to `send` are used as the parameters to the called method. – Arsen7 Feb 28 '14 at 12:48
  • I disagree with this: "`send` may be as dangerous as `eval` if you do not **sanitize your input**". If he uses your code, then it could only call a method belonging to `QuestionProcessor` that starts with "get". Unless he codes one of those methods to execute arbitrary code, how could that be dangerous? – Max Feb 28 '14 at 15:11
  • In this case it _looks_ safe, I agree. But if you use any gem, framework, or anything, it may add some methods to Object, Kernel, or anything, and you may end up calling these. In ruby you cannot be always sure what methods the object really has defined, thus I would prefer to be cautious. So, my warning was meant to be a little more like a general-purpose advice. – Arsen7 Feb 28 '14 at 17:12
3

There is a function in ruby for exactly this reason, the send function. It is part of the Object class so everything has it.

read more here: http://ruby-doc.org/core-2.1.1/Object.html#method-i-send

for metaprogramming I recommend you read this whole tutorial: https://rubymonk.com/learning/books/2-metaprogramming-ruby/

keeslinp
  • 176
  • 1
  • 5