Ruby abstraction

Question

I'm new to Ruby, coming primarily from C# and ActionScript 3 (among other langauges). I'm curious about abstracting functionality. Specifically, wrapping and abstracting Ruby's FTP and SFTP libs.

I was searching around and came across a gem called Backup. It really got my attention because it supports backing stuff up via S3, SCP, SFTP and FTP. So I thought, "wow, here's a perfect example!" I started browsing the source, but then I came across code like:

case backup.procedure.storage_name.to_sym
  when :s3    then records = Backup::Record::S3.all   :conditions => {:trigger => trigger}
  when :scp   then records = Backup::Record::SCP.all  :conditions => {:trigger => trigger}
  when :ftp   then records = Backup::Record::FTP.all  :conditions => {:trigger => trigger}
  when :sftp  then records = Backup::Record::SFTP.all :conditions => {:trigger => trigger}
end

view the full source on GitHub

It's littered with case/when statements! If I were attacking this in C#, I'd write a Protocol interface (or abstract class) and let FTP and SFTP implement it. Then my client class would just pass around an instance of Protocol without caring about the implementation. Zero switch/cases.

I'd appreciate a little guidance on best practices in this situation when coding in Ruby.

Answer 1

And you could have done it that way in Ruby, too

Because of the dynamic typing Ruby doesn't need interfaces. For that matter, it doesn't need prototypes, signatures, or templates and even subclasses, while present, are not strictly necessary.

And when I say "doesn't need", I just mean that the design patterns you are referring to can be directly implemented in Ruby. Because no calling restrictions are enforced at "compile-time", any design pattern that depends on interfaces or any flavor of polymorphism is directly available in Ruby.

Yes, it doesn't appear as if that package took full advantage of the possible abstractions, but perhaps (a) it doesn't matter, as long as it works. After all, you didn't need to type it in, or (b) there is some not-immediately-obvious benefit to the simple compositional pattern that is used.

Answer 2

To be fair, the code above is very explicit. Apart from implementing it as a hierarchy of objects, you could go nuts and do something like

storage_method = backup.procedure.storage_name.upcase

records = eval("Backup::Record::#{storage_method}.all :conditions => {:trigger => trigger}"

I'm not suggesting that this is actually correct but hopefully it illustrates my point that concise code isn't always better than explicit code. Ruby can be just as dangerous as C :-)

Answer 3

There might well be a better way, but this is my shot.

ALLOWD_OPTIONS = [:s3, :scp, :ftp ,:sftp].freeze
type = ALLOWD_OPTIONS.detect { |e| e == backup.procedure.storage_name.to_sym }
if type
  records = send(%s"Backup::Record::#{type.upcase}.all", :conditions => {:trigger => trigger})
end

or

if [:s3, :scp, :ftp, :sftp].include?(backup.procedure.storage_name.to_sym)
   records = __send__ (%s"Backup::Record::#{backup.procedure.storage_name.to_sym.upcase}.all", 
                       :conditions => {:trigger => trigger})
end

You could use public_send() for extra security if you are using Ruby 1.9. I looked at the file in question. There are many case statements. You could make a private method to do something like I wrote above to minimize duplication.

ALLOWD_OPTIONS = [:s3, :scp, :ftp ,:sftp].freeze

records = send_method_with_type("all", backup.procedure.storage_name.to_sym, 
                                       :conditions => {:trigger => trigger})

private

def send_method_with_type(method, type, *args)
  raise unless ALLOWD_OPTIONS.inlucde? type
  send(%s"Backup::Record#{type.upcase}.#{method}", args)
end

Answer 4

There's a couple of ways you can do it elegantly I think. One, is to use send as TK suggested above. The other is to use "method_missing", the method Ruby calls when it can't find an existing method.

Metaprogramming Ruby has great coverage of both of these options. Luckily enough, it's in the free sample chapter online (I recommend the book, if you want to learn more).

Apologies for not giving you a code snippet but have a read through that and see if it helps.

Answer 5

Most of the time, case expressions in an OO language are a sign that you're not using polymorphism correctly. In this case, I would make it something like:

backup.procedure.storage_class.all :conditions => {:trigger => trigger}

Where storage_class returns the appropriate class. (Actually, I would prefer to make storage_class a property of the backup itself, but I don't know whether that's practical in the design of this library.)