Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it acceptable practice to patch Ruby's base classes, such as Fixnum?

I am still very new to Ruby (reading through the Pickaxe and spending most of my time in irb), and now that I know it's possible to patch classes in Ruby, I'm wondering when it's acceptable to do so, specifically whether it's acceptable to patch Ruby's base classes. For example: I answered another Ruby question here where the poster wanted to know how to subtract hours from a DateTime. Since the DateTime class doesn't seem to provide this functionality, I posted an answer that patches the DateTime and Fixnum classes as a possible solution. This is the code I submitted:

require 'date'

# A placeholder class for holding a set number of hours.
# Used so we can know when to change the behavior
# of DateTime#-() by recognizing when hours are explicitly passed in.

class Hours
   attr_reader :value

   def initialize(value)
      @value = value
   end
end

# Patch the #-() method to handle subtracting hours
# in addition to what it normally does

class DateTime

   alias old_subtract -

   def -(x) 
      case x
        when Hours; return DateTime.new(year, month, day, hour-x.value, min, sec)
        else;       return self.old_subtract(x)
      end
   end

end

# Add an #hours attribute to Fixnum that returns an Hours object. 
# This is for syntactic sugar, allowing you to write "someDate - 4.hours" for example

class Fixnum
   def hours
      Hours.new(self)
   end
end

I patched the classes because I thought in this instance it would result in a clear, concise syntax for subtracting a fixed number of hours from a DateTime. Specifically, you could do something like this as a result of the above code:

five_hours_ago = DateTime.now - 5.hours

Which seems to be fairly nice to look at and easy to understand; however, I'm not sure whether it's a good idea to be messing with the functionality of DateTime's - operator.

The only alternatives that I can think of for this situation would be:

1. Simply create a new DateTime object on-the-fly, computing the new hour value in the call to new

new_date = DateTime.new(old_date.year, old_date.year, old_date.month, old_date.year.day, old_date.hour - hours_to_subtract, date.min, date.sec)


2. Write a utility method that accepts a DateTime and the number of hours to subtract from it

Basically, just a wrapper around method (1):

def subtract_hours(date, hours)
  return DateTime.new(date.year, date.month, date.day, date.hour - hours, date.min, date.sec)
end


3. Add a new method to DateTime instead of changing the existing behavior of #-()

Perhaps a new DateTime#less method that could work together with the Fixnum#hours patch, to allow syntax like this:

date.less(5.hours)

However, as I already mentioned, I took the patching approach because I thought it resulted in a much more expressive syntax.

Is there anything wrong with my approach, or should I be using one of the 3 alternatives (or another one I haven't thought of) in order to do this? I have the feeling that patching is becoming my new 'hammer' for problems in Ruby, so I'd like to get some feedback on whether I'm doing things the "Ruby way" or not.

like image 822
Mike Spross Avatar asked Oct 27 '08 01:10

Mike Spross


2 Answers

My personal answer, in a nutshell: the core-class patching hammer should be at the bottom of your toolbox. There are a lot of other techniques available to you, and in almost all cases they are sufficient, cleaner, and more sustainable.

It really depends on the environment in which you are coding, though. If it's a personal project - sure, patch to your heart's content! The problems begin to arise when you are working on a large codebase over a long period of time with a large group of programmers. In the organization I work for, which has Ruby codebases of over 100KLOC and and twenty or so developers, we have started to crack down pretty hard on monkey patching, because we've seen it lead to head-scratching, man-hour wasting behavior far too often. At this point we pretty much only tolerate it for temporarily patching third-party code which either hasn't yet incorporated or won't incorporate our source patches.

like image 97
Avdi Avatar answered Nov 01 '22 04:11

Avdi


Personally, I think it's acceptable to add methods to the base classes, but unacceptable to modify the implementation of existing methods.

like image 6
Dónal Avatar answered Nov 01 '22 05:11

Dónal