gistfile1.rb

# Between those alternatives I'd go with this one.
#
# Ruby has open classes, there is nothing wrong about embracing them. Even Rails that used to
# create modules and mix them into the classes has decided to change it on rails3 and simply
# open the classes and add the methods.
#
# It's simple, and it works. Of course you shouldn't be adding stuff that are specific to 
# your application's business logic.
class Fixnum
  def to_bit_array
    to_s(2).reverse.split("").map(&:to_i)
  end
end

# Which is why I actually would change this method. Since a Fixnum is a sequence of bytes 
# already and you just need to check if some bits are on, then I would change the method to
# this:
class Fixnum
  # Returns true if all the given bits positions are on (set to 1)
  # Example:
  #   0b001110.bits_on?(1, 2, 3)
  def bits_on?(*bits)
    bits.all? { |n| self[n] == 1 }
  end

  # Adds a singular alias
  alias_method :bit_on?, :bits_on?
end

# If you actually need to perform more calculations over the sequence of bits, I'd create a 
# separate class for it but using composition instead of inheritance.
#
# Example:
#   m = BitMap.new(0b001110)
#   m.bits_on # => [1, 2, 3]
#   m.bits_on?(1,2,3) # => true
class BitMap
  def initialize(integer)
    @integer = integer
  end

  # Returns true if all the given bits positions are on (set to 1)
  # Example:
  #   0b001110.bits_on?(1, 2, 3)
  def bits_on?(*bits)
    bits.all? { |n| @integer[n] == 1 }
  end

  # Adds a singular alias
  alias_method :bit_on?, :bits_on? 

  # Return all the bits that are on
  # Example:
  #   0b001110.bits_on # => [1,2,3]
  def bits_on
    indexes = []
    (0...@integer.size*8).each { |n| indexes << n if @integer[n] == 1 }
    indexes
  end
end

Whether Rails opens the classes directly or mixes modules into them is largely a procedural difference. At the end of the day, you're still adding methods to the class.

Ruby has open classes, there is nothing wrong about embracing them.

That's true. But there are times when it's appropriate to use them, and times when it isn't. I don't think this is the appropriate place to use them.

I think the good use cases for open classes are the following:

• A library like ActiveSupport, which "fills in the missing blanks" in the Ruby standard library, and adds backwards-compatibility methods so that you can use Ruby 1.9.1 features like Symbol#to_proc and Hash#map in Ruby 1.8.7.
• A gem with a narrow focus, such as an advanced math library that adds methods to Fixnum and Float. If a library like this adds methods to the base classes, it's not surprising.
• A private application in which you and a few other people that you know are the primary developers. In other words, most Rails applications. Here, you have complete control over which gems are included in your app, and so if there's a conflict you just fix it.

Now here's why I think your code above is not a good example of when to open a class and write methods in it. Imagine if, in the name of "clean design," most gem authors wrote their utility methods into String, Fixnum, Array, etc. The likelihood of conflicts would be very high for any app that includes lots of gems. Just imagine how annoying it would be if you require my linguistics gem, and it conflicted with say, a JPEG processing gem that also adds a "bits_on" method to Fixnum, but with slightly different parameters or return values. If enough people followed your advice, that would be a realistic scenario.

The other scenario you describe; creating a new class and delegating most of the functionality to the Fixnum, is actually one that I may end up using in my situation. It often works. But in situations where performance counts, you're incurring a lot of expense by allocating a new object, and that expense gets compounded when you want to have methods return instances of the wrapping class rather than the wrapped one:

class SpanishString
  def hola_mundoize
    self << "... y ¡hola, mundo!"
  end
end

ss = SpanishString.new("hello")
ss.hola_mundoize.class # String

# Ok, now that's a problem, because now if I want to invoke any custom methods I
# can't, because it's not an instance of SpanishString. So let's do this:

class SpanishString
  def hola_mundoize
    self.class.new(self << "... y ¡hola, mundo!")
  end
end

Here, in the course of invoking methods on this class, I'm going to be making a lot of instances of a wrapper class, and that produces garbage that will need to be collected. In situations where performance counts, this can be unacceptable. Having the utility method in a namespace instead creates less garbage:

module SpanishString
  def self.hola_mundoize(string)
    string << "... y ¡hola, mundo!"
  end
end

SpanishString.hola_mundoize("my string")

Yes, this is ugly. I don't like the way it looks. But if you benchmark it against the delegator, you see numbers like this:

# encoding: utf-8
require "rbench"

module SpanishString
  def self.hola_mundoize(string)
    string << "... y ¡hola, mundo!"
  end
end

class SpanishString2

  attr :string

  def initialize(string)
    @string = string
  end

  def hola_mundoize
    self.class.new(string << "... y ¡hola, mundo!")
  end
end

RBench.run do
  column :utility_method
  column :delegator

  report "run", 1_000_000 do
    utility_method { SpanishString.hola_mundoize("hello world") }
    delegator { SpanishString2.new("hello world").hola_mundoize }
  end

end

# Results:
#
#             UTILITY_METHOD | DELEGATOR |
# ----------------------------------------
# run                  0.891 |     1.966 |

So there you have two pretty legitimate reasons for putting utility methods in modules: avoidance of conflict, and performance. If you look through the Rails source, they're all over the place too, precisely for those reasons.

Of course this is just a contrived example, and a 1 second difference over a million method calls is not that huge. But if you're working with more complex objects the difference will be much bigger.

So this is why I'd like to have protected for methods like this - because sometimes I need them, but they are ugly and I don't want them polluting my API.

Okay, I tend to agree with you about adding the method directly to the class. Namespace conflicts are likely to occur but I also think using this approach is OK if you're cautious about it: only adds methods that are actually extensions to the classes and you have control over the application environment.

Since integers are historically used to represent a string of bits, I still think that make senses to have bit related methods on Fixnum, but again, in a controlled environment.

Anyway, in a application of mine I would definitely use my latest example where I create a separate class using composition (and possible, delegation). Here is why and some counter-opinions on your arguments:

But in situations where performance counts, you're incurring a lot of expense by allocating a new object, and that expense gets compounded when you want to have methods return instances of the wrapping class rather than the wrapped one:

If performance is a problem then sometimes you need to sacrifice the design and choose a different approach and if it is really a problem it is probably a better idea not to use an object oriented language and choose another one that is faster (C, Go, Scala, whatever).

Here, in the course of invoking methods on this class, I'm going to be making a lot of instances of a wrapper class, and that produces garbage that will need to be collected. In situations where performance counts, this can be unacceptable. Having the utility method in a namespace instead creates less garbage:

Yes, depending on how you implement the class you will be creating a lot of instances but you don't necessarily needs to create a instance for each method call. Just stop thinking about it as a wrapper and think of it as a actually behavioral class and you'll see that there will be only a few (if none) methods that will require you to return another instance of the same class. Take the String class, for example: it also creates a new instance of String when some methods are called (i.e. String#+) but it also has destructive methods that change the instance (i.e. String#<<).

Also, I think your example wasn't really appropriate to compare to my BitMap implementation, since your SpanishString seems more like a text decorator.

Another reason to use a separate class is that it's easy to change the representation of your bit string, using the power of duck typing. Assuming the variable @integer is now named @bitstring ;), consider the following:

m = BitMap.new([0, 1, 1, 1, 0])
m.bits_on # => [1, 2, 3]
m.bits_on?(1,2,3) # => true

Anyway, I don't think you shouldn't use module methods, but usually the module methods I create are to be called from objects outside the namespace to perform a certain action that uses classes/definitions from within the module. For example if you have a complex set of classes inside the namespace and you have a very common use case of the classes: instead of creating and setting up the instances for each time you need it, you'd wrap that on a module method and call that method.

About the performance issue, if you're really worried about optimization, here is benchmark of your module method implementation against mine:

require 'benchmark'

module Phonology
  def self.bit_array(int)
    int.to_s(2).reverse.split("").map(&:to_i)
  end
end

class BitMap
  def initialize(integer)
    @integer = integer
  end

  def bits_on
    indexes = []
    (0...@integer.size*8).each { |n| indexes << n if @integer[n] == 1 }
    indexes
  end
end

Benchmark.bmbm do |x|
  x.report("Module method :") { Phonology.bit_array(0b1110) }
  x.report("BitMap class  :")  { BitMap.new(0b1110).bits_on }
end

And here are two results:

Rehearsal ---------------------------------------------------
Module method :   0.000000   0.000000   0.000000 (  0.000059)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000038)
------------------------------------------ total: 0.000000sec

                      user     system      total        real
Module method :   0.000000   0.000000   0.000000 (  0.000038)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000032)

Rehearsal ---------------------------------------------------
Module method :   0.000000   0.000000   0.000000 (  0.000062)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000038)
------------------------------------------ total: 0.000000sec

                      user     system      total        real
Module method :   0.000000   0.000000   0.000000 (  0.000023)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000030)

Both are fasts and pretty close to each other, even creating a new instance. That's because I'm using the Fixnum#[] operator instead of doing string conversions and transforming to array. So, make sure what to optimize ;)

Oh, and if the bit string was bigger then transforming the integer to a string, then to an array, then reverse, then map would definitely take longer.

Using the number 0xffffffffffffffffffffffff:

Rehearsal ---------------------------------------------------
Module method :   0.000000   0.000000   0.000000 (  0.000200)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000121)
------------------------------------------ total: 0.000000sec

                      user     system      total        real
Module method :   0.000000   0.000000   0.000000 (  0.000129)
BitMap class  :   0.000000   0.000000   0.000000 (  0.000054)