A few Ruby Anti-Patterns

Some things you can do with Ruby you just shouldn’t. Often one doesn’t notice until later on, when you are looking for a bug in code.

rescue everything

This is a story from real life. In our recent release of net-ldap we have this snippet of code:

  begin
    @socket = TCPSocket.new(host, port)
  rescue
    raise LdapError, "Something is wrong with your connection."
  end

So when you get the ‘Something is wrong …’ error, where do you go look? Right, you check and recheck your credentials.

Except that wasn’t the problem. We recently reorganized the project and fixed some Ruby 1.9 stuff. This was the reason for our rerelease as well. During that work, we (ok, probably I did) fat-fingered the import of ‘socket’ with the obvious NameError as a consequence.

The above code will tell you that something is wrong with the connection even if nothing is. Look at how I think this should be written:

  begin
    @conn = TCPSocket.new( server[:host], server[:port] )
  rescue SocketError
    raise LdapError, "No such address or other socket error."
  rescue Errno::ECONNREFUSED
    raise LdapError, "Server #{server[:host]} refused connection on port #{server[:port]}."
  end

We could dispute the details here, but the main point is that you should almost never rescue everything. This is an anti-pattern in Ruby and in other languages as well. If you don’t know what happened, make it scream really loud.

There is another big anti-pattern hidden in this war story: Write tests for your code. Because that would have told me what was wrong.

Extending basic Types

Let’s say I have a few derivates of Ruby basic types with strings attached:

  class StringString < String
    attr_accessor :strings
  end
  class StringArray < Array
    attr_accessor :strings
  end
  
  ary = [
    StringString.new('foo'),
    StringArray.new([StringString.new('bar')])
  ]

I want to create a clean YAML representation of this, looking something like

  ---
  - foo
  - - bar

But turning what I have into YAML I get this:

  --- 
  - !str:StringString foo
  - !seq:StringArray 
    - !str:StringString bar

And of course, turning it to YAML is just one of the problems you have. Everywhere you use inspection/reflection in your code, these StringString things won’t behave correctly.

Stop and think about how you would do it for a second. There seems to be no real easy way to do this, even when you own the StringString and StringArray types. Of course, if you knew you only had strings, you could use #to_s / #to_a and be done with it. But polymorphically? What we need is a #detach_strings method.

  def detach_strings(mess)
    mess.class.ancestors[1].new(mess)
  end

  puts detach_strings(StringString.new('bar')).to_yaml
  # => "--- bar\n"

Ugly, isn’t it? Yeah. But in my opinion that is entirely the blame of the extended basic type anti-pattern. I wont spend time on making this beautiful, but rather will transform the code so it doesn’t use extended types anymore.

Your own Anti-Patterns?

Do you have any Ruby anti-patterns from your work? I would be interested in hearing about them.