views:

254

answers:

2

In fairly large Ruby application, we have a situation where a given object is identified by a couple of things: name and id, say. Each of these value types serves a somewhat different purpose and so are not exactly equivalent (id and name persist in different places). So we wind-up with a variety of values being passed around the application (ids, names and objects). This situation seems like a problem to at least some degree since we've already been bitten by bugs which involved not being clear what type should be passed to a given function. I actually recall seeing similar problem code in a number of applications over the years, though I again never gave a specific name to it.

Ruby, as a typeless language, does not allow classical type-based polymorphic functions in the way that C++ does. As a work around, a colleague has often resorted code of this sort:

  def initialize starting_value
    if starting_post.kindof? Foo
      @starting_id = get_id_from_foo starting_value
    elsif starting_post.kindof? Bar
      @starting_id = get_id_from_bar starting_value
    else
      raise "illegal type"
    end
  end

The proliferation of this code around our code-base (not just initializers) results in what I would call "messy-polymorphism". It often works but sometimes creates very puzzling situations.

I have three questions about this.

  • Is there a formal name for this as an anti-pattern? "Messy Interface?", "Messy Polymorphism?" or something else?
  • How bad to do people think this is?
  • Is there a systematic way to refactor this? The challenge that we have with ordinary refactoring that many tests we're created use this loose-typing and thus we would have to change both the tests and the implementations simultaneously and so wouldn't have the scaffold effect of ordinary test-based refactoring. I am think that one could actually "strengthen" this loose polymorphism, and abstract-out the code into a function rather than immediate rip it out. But would this be a good idea?
+6  A: 

Can't you define some sort of get_id method so that an id returns itself, an object returns the id, and the name does whatever it needs to do to get an id? Then you can always canonicalise anything which you know will be one of the three. Likewise with get_name and get_object methods if those are needed.

That is to say, you've defined an implicit ThingWhatHasAnID interface, and made that the duck-type of your function parameter.

Unless I'm missing something, I'd call this anti-pattern "missed opportunity to create an abstraction".

Steve Jessop
+3  A: 

Almost anytime you find yourself switching on an object's class, it's a clue that the behavior should be a method of the object itself. Message dispatch is polymorphic based on the receiver. In this case, it should be something like:

def initialize starting_value
  @starting_id = starting_value.id
end

Define id to do whatever the various get_id_from_* methods used to do. The illegal type case will already raise because you'll get a NoMethodError.

As for what to call this, I'd call it "procedural programming in an OO language."

Chuck