Topic: Help with refactoring

I have this method in a model which holds downloaded files to check the extension of an uploaded file and choose an icon which is applicable to that file type:

  def file_thumb
    case extension
    when /pdf$/i
      thumb = 'pdf.gif'
    when /(ai|eps)$/i
      thumb = 'ai.gif'
    when /(doc|txt|rtf|rdf)$/i
      thumb = 'doc.gif'
    when /gif$/i
      thumb = 'gif.gif'
    when /jpg$/i
      thumb = 'jpg.gif'
    when /mov$/i
      thumb = 'mov.gif'
    when /png$/i
      thumb = 'png.gif'
    when /psd$/i
      thumb = 'psd.gif'
    when /(html|php|rb|rhtml)$/i
      thumb = 'web.gif'
    else
      thumb = "Generic.gif"
    end
   
    result = ICON_DIR + thumb
  end

Seems a bit long-winded, but my skills at refactoring are not so great and I can't seem to find a shorter and more concise way of writing it. Can anyone help and suggest and different options?

Thanks for any help.

Re: Help with refactoring

Well, it looks like the large majority of them could be handled by simply taking the extension and appending .gif.  However you have a number of exceptions, so you could do something like this.

exceptions = {
  :eps => 'ai',
  :txt => 'doc',
  :rtf => 'doc',
  :rdf => 'doc'
}

if exceptions.has_key? extension
  thumb = exceptions[thumb.to_sym] + '.gif'
else
  thumb = extension + '.gif'
end

Re: Help with refactoring

cheers for the help. It doesnt seem to work though.. a file with the extension of txt ends up with a icon named txt.gif rather than doc.gif?

Re: Help with refactoring

You probably need to change:

if exceptions.has_key? extension

to

if exceptions.has_key? extension.to_sym

Railscasts - Free Ruby on Rails Screencasts

Re: Help with refactoring

oops, code slip on my part.  Sorry, I didn't test it.

exceptions = {
  'eps' => 'ai',
  'txt' => 'doc',
  'rtf' => 'doc',
  'rdf' => 'doc'
}

if exceptions.has_key? extension
  thumb = exceptions[extension] + '.gif'
else
  thumb = extension + '.gif'
end

Re: Help with refactoring

okay, now it raises an error:

You have a nil object when you didn't expect it!
The error occured while evaluating nil.to_sym

Re: Help with refactoring

idlefingers wrote:

okay, now it raises an error:

You have a nil object when you didn't expect it!
The error occured while evaluating nil.to_sym

Sounds like the extension variable is nil. If you want to handle that case, you can do:

if extension.nil?
  thumb = 'generic.gif' # or whatever you want
elsif exceptions.has_key? extension.to_sym
  thumb = exceptions[extension.to_sym] + '.gif'
else
  thumb = extension + '.gif'
end

If you're using strings as the keys as Ben demonstrated in his last post, remove the to_sym conversion.

Last edited by ryanb (2006-10-02 17:28:07)

Railscasts - Free Ruby on Rails Screencasts

Re: Help with refactoring

the problem was that I had 'thumb' where 'extension' should be.

Re: Help with refactoring

ah.. I didn't notice the exceptions[thumb.to_sym] needing to be exceptions[extension.to_sym]

I had modified it to handle the generic icon, but that's even better, ryan. Thanks.

Thanks for both of your help. Much appreciated.