Block Your Privates!

Posted by Rick DeNatale Tue, 04 Sep 2007 15:28:00 GMT

I’ve noticed that some rubyists like to use indentation to delineate method visibility.

The first time I noticed this was when Marcel Molina Jr. used it during the charity testing workshop at the Ruby Hoedown.. I just encountered it again in at least one piece of sample code from Rob Orsini’s “Rails Cookbook.”

During the testing workshop, Chad Fowler expressed displeasure with this style, and I’ve got to agree.

Here’s an example of this style:

class Gadget
  def framilize
    razzlitize if options[:secret_ingredient_6X]
  end

  private
    def razzlitize
    end
end

The problem is that the indentation is “artificial”, there really is no nesting of evaluation scope here. The way that Module#private and it’s kin work when they are called without arguments is to set a marker onto Ruby’s eval stack which gets popped when the current evaluation context is exitted. So the effects of the private method remain in effect until either the end of the current context, or one of the other methods like public or protected, whichever comes first. This means that if we extend the above example:

class Gadget
  #...
  private
    def razzlitize
    end

  def dazzlitize
  end
end

The indentation can trick us into thinking that dazzlitize is public when it’s really private.

I’d always found the nature of methods like private without arguments to be a minor annoyance with Ruby syntax. In my humble opinion they should have been able to take a block which delineated their effect. What if we could write:

class Gadget
  #...
  private do
    def razzlitize
    end
  end

  def dazzlitize
  end
end

Well, actually we can write that, but it doesn’t work as expected, since the block is silently ignored, and the method doesn’t even get defined.

So I started doing a little metaprogramming, to fix this. It seemed obvious how to do this. You alias_method Module#private so you can extend it, redefine the method to check for a block and if none is given, call the original method, otherwise evaluate the block in a way which makes any methods defined private. And of course you use test driven design to do this.

I made some progress but ran into some problems:

The obvious way to evaluate the block:
```
class module
  alias :old_private, :private
  def private(*args, &blk)
    if blk_given?
       module_eval do
         old_private
         blk.call
       end
    else
      old_private(*args)
    end
end
```
Doesn’t seem to work. The method isn’t made private. I had to resort to getting a list of the methods before the block, then calling old_private with the difference between the new methods after the block is evaluated, and those before.

The module_eval now looked something like this:
```
module_eval do
  existing = instance_methods(false)
  blk.call
  new_methods = instance_methods(false)-existing
  old_private(new_methods.map {|m| m.to_sym}
```
There are some obvious shortcomings in this code, but at this point, the goal was to do the simplest thing which could work for the test cases at hands, the complications would be dealt with later.
To add insult to injury, calling old_private from within the new definition doesn’t work either, the evaluation context gets marked as described above, but it gets popped off when we exit the context of the new definition. At this point I decided to punt, temporarily at least, and just define a new method Module#with_private which took a block, and leave the existing Module#private alone.
Then I decided that I should test changing the visibility of an existing method, in other words a test case like this:
```
class Test
  protected
  def meth;end
  with_private do
     def meth;end
  end
end
```
Test#meth should end up private, but my simple diff failed to notice the definition, things were starting to get more complicated. It looked like I’d need to hook Module#method_added and probably others. I began to wonder whether it was worth it.

Then I started to ponder the fact that the effect of private/protected/public gets popped when the evaluation context exits. So I said to myself, “Is there a way to take advantage of that?” Well, Virginia, yes there is.

class Gadget
  #...
  class_eval do
    private

    def razzlitize
    end

  end

  def dazzlitize
  end
end

This seems to work as expected, here’s a link to a test case file.

Now I still prefer being able to give a block to Module#private and it’s ilk, and I might still work on that, but in the meantime, class_eval and module_eval seem to provide a way to actually do what I want.

Posted in ruby, best_practices | Tags metaprogramming, style | 6 comments | no trackbacks

Comments

nuke edit
Daniel Berger said 42 minutes later:

If def returned something useful we could just do:

private def foo ... end

But, as the result of analysis paralysis, we get nothing. Boo.
nuke edit
Robert Dober said 1 day later:

Hey Rick, great to hear from you after some time, maybe you want to have a look at my shot at your problem. Quite an ugly piece of code, but maybe it is useful to you.

Cheers Robert

Here we go:

class Module alias_method :private, :private @isPrivate = [ false ] class << self attr_reader :isPrivate end end
```
def private *args, &blk
  return private( *args ) unless blk
  Module.isPrivate.push true
  class_eval &blk
  Module.isPrivate.pop
end   
def method_added *args, &blk
  private args.first if Module.isPrivate.last
end
```
class A def initialize puts pm end private do def pm; 46 end end end

puts A.new.pm

Robert Dober said 1 day later:

Hmm did not format

let me try again

class Module
   alias_method :private, :priavte
   @isPrivate = [ false ]
   class << self
      attr_reader :isPrivate
   end
   ...

Robert Dober said 1 day later:

Sorry Rick I did not see the Preview button, bad bad me, let me give it a last shot:

class Module
 alias_method :old_private, :private
 @isPrivate = [ false ]
 class << self
   attr_reader :isPrivate
 end
end

def private *args, &blk
  return old_private( *args ) unless blk
  Module.isPrivate.push true
  class_eval &blk
  Module.isPrivate.pop
end   
def method_added *args, &blk
  old_private args.first if   Module.isPrivate.last
end

class A
 def initialize
   puts pm
 end
 private do
   def pm; 46 end
 end
end

puts A.new.pm

nuke edit
Rick DeNatale said 1 day later:

Robert,

Thanks. I was thinking along those lines, but got tired. ;-)
I’m not sure why
```
  return old_private(*args)
```
Seems to work for the no block given case when:
```
  old_private(*args)
```
As the last expression doesn’t seem to.

This does look like it could be the basis for a real implementation of the idea.
nuke edit
Rick DeNatale said 1 day later:

Oops Robert,
It doesn’t work after all. There was a misplaced end which confused me:
```
class Module
 alias_method :old_private, :private
 @isPrivate = [ false ]
 class << self
   attr_reader :isPrivate
 end
#  end <==== This end

 def private *args, &blk
  return old_private( *args ) unless blk
  Module.isPrivate.push true
  class_eval &blk
  Module.isPrivate.pop
 end   
 def method_added *args, &blk
  old_private args.first if Module.isPrivate.last
 end
# SHOULD BE HERE
end

class A
 def initialize
   puts pm
 end
 private do
   def pm; 46 end
 end
end

puts A.new.pm
```
Now while the example you gave works, the use of private with no arguments or block is still broken.
```
class B
   private
   def priv
   end
end
```
Gives B.private_instance_methods(false) => []

and B.public_instance_methods(false) => [‘priv’]

The problem still seems to be that if you execute private with no args inside a method, its effect gets popped off the stack when you return, and so it has no effect.

And I can’t see a way to stack it when there’s no block and no args and force it with method_added, since there’s doesn’t seem to be a way to know when the class definition scope is exitted and therefore when to pop it.