tags:

views:

142

answers:

3

I'm trying to redefine the File.dirname method to first change %20s to spaces. But the following gives me an error

class File
   old_dirname = instance_method(:dirname)    

   define_method(:dirname) { |s|
       s = s.gsub("%20"," ")
       old_dirname.bind(self).call(s)
   }
end

This trhows a NameError exception: undefined method 'dirname' for class 'File'

What is the right way to do this?

+1  A: 

dirname is a class method of File, not an instance method, so you're just defining a new instance method. Also, the idiomatic way to alias a method is with alias. So:

class <<File
  alias old_dirname dirname
  def dirname(f)
    old_dirname(f.gsub("%20", " "))
  end
end

The class <<whatever syntax adds methods to an individual object — in this case, the File class.

Chuck
thanks for the help
jrhicks
This is *not* equivalent to the code @jrhicks posted in his question! This code pollutes the `File` metaclass's namespace with a residual `File::old_dirname` method, whereas the original code in the question takes great care to avoid this issue.
Jörg W Mittag
The discussion of scope is entirely correct, Chuck, but Jörg is right: `alias_method` may be "idiomatic," but it's definitely less safe than the OP's original approach.
James A. Rosen
+3  A: 

Just be careful.

You're changing the behaviour of the method, not just its implementation. This is generally poor practice, because it weakens the value of the API as a dependable contract.

Instead, consider transforming the input closer to the point of receipt.

sheldonh
+4  A: 

As Chuck already wrote, File::dirname is a singleton method of the File class object (or more precisely an instance method of the File class object's metaclass), not an instance method of the File class.

So, you have to open up File's metaclass, not the File class itself:

#!/usr/bin/env ruby

class << File
  old_dirname = instance_method :dirname

  define_method :dirname do |*args|
    old_dirname.bind(self).(*args).gsub '%20', ' '
  end
end

require 'test/unit'
class TestFileDirname < Test::Unit::TestCase
  def test_that_it_converts_percent20_to_space
    assert_equal '/foo bar/baz', File.dirname('/foo%20bar/baz/quux.txt')
  end
end

However, I agree with @sheldonh: this breaks the API contract of File::dirname.

Jörg W Mittag