1
votes

Due to the fact that Ruby doesn't support overloading (because of several trivial reasons), I am trying to find a way to 'simulate' it.

In static typed languages, you mustn't use instanceof, (excepting some particular cases of course...) to guide the application.

So, keeping this in mind, is this the correct way to overload a method in which I do care about the type of the variable? (In this case, I don't care about the number of parameters)

class User
  attr_reader :name, :car
end

class Car
  attr_reader :id, :model
end

class UserComposite
  attr_accessor :users

  # f could be a name, or a car id
  def filter(f)
    if (f.class == Car)
      filter_by_car(f)
    else
      filter_by_name(f)
    end
  end

  private

  def filter_by_name(name)
    # filtering by name...
  end

  def filter_by_car(car)
    # filtering by car id...
  end
end
3

3 Answers

4
votes

Personally, I don't see a big problem in branching that way. Although it would look cleaner with a case

def filter(f)
  case f
  when Car
    filter_by_car(f)
  else
    filter_by_name(f)
  end
end

Slightly more complicated example involves replacing branching with objects (ruby is oop language, after all :) ). Here we define handlers for specific formats (classes) of data and then look up those handlers by incoming data class. Something along these lines:

class UserComposite
  def filter(f)
    handler(f).filter
  end

  private
  def handler(f)
    klass_name = "#{f.class}Handler"
    klass = const_get(klass_name) if const_defined?(klass_name)
    klass ||= DefaultHandler
    klass.new(f)
  end

  class CarHandler
    def filter
      # ...
    end
  end

  class DefaultHandler # filter by name or whatever
    def filter
      # ...
    end
  end
end
5
votes

There are cases where this is a good approach, and Ruby gives you the tools to deal with it.

However your case is unclear because your example contradicts itself. If f.class == Car then filter_by_car accepts a _car, not a _car_id.

I'm assuming that you're actually passing instances of the class around, and if so you can do this:

# f could be a name, or a car
def filter(f)
  case f
  when Car
    filter_by_car(f)
  else
    filter_by_name(f)
  end
end

case [x] looks at each of its when [y] clauses and executes the first one for which [y] === [x]

Effectively this is running Car === f. When you call #=== on a class object, it returns true if the argument is an instance of the class.

This is quite a powerful construct because different classes can define different "case equality". For example the Regexp class defines case equality to be true if the argument matches the expression, so the following works:

case "foo"
when Fixnum
  # Doesn't run, the string isn't an instance of Fixnum
when /bar/
  # Doesn't run, Regexp doesn't match
when /o+/
  # Does run
end
0
votes

There could be a problem lurking in your architecture - UserComposite needs to know too much about Car and User. Suppose you need to add more types? UserComposite would gradually become bloated.

However, it's hard to give specific advice because the business logic behind filtering isn't clear (architecture should always adapt to your real-world use-cases).

Is there really a common action you need to do to both Cars and Users?

If not, don't conflate the behavior into a single UserComposite class.

If so, you should use decorators with a common interface. Roughly like this:

class Filterable
  # common public methods for filtering, to be called by UserComposite
  def filter
    filter_impl  # to be implemented by subclasses
  end
end

class FilterableCar < Filterable
  def initialize(car)
    @car = car
  end
  private
  def filter_impl
    # do specific stuff with @car
  end
end

class DefaultFilterable < Filterable
  # Careful, how are you expecting this generic_obj to behave?
  # It might be better replace the default subclass with a FilterableUser.
  def initialize(generic_obj)
    # ...
  end
  private
  def filter_impl
    # generic behavior
  end
end

Then UserComposite only needs to care that it gets passed a Filterable, and all it has to do is call filter on that object. Having the common filterable interface keeps your code predictable, and easier to refactor.

I recommend that you avoid dynamically generating the filterable subclass name, because if you ever decide to rename the subclass, it'll be much harder to find the code doing the generating.