Friday, August 31, 2007

Ruby: Adding a "not" method for readability

The other day I was working with an if statement that looked similar to the snippet below.

if !response.incomplete? && !response.invalid? && response.total > 0
...
end

I spent enough time in C, C#, etc to be able to parse the if statement fairly easily; however, I thought it would improve readability if I could write the following snippet instead.

if response.not.incomplete? && response.not.invalid? && response.total > 0
...
end

Adding a "not" method to Object turned out to be fairly easy. I started with the following tests (block syntax provided by dust).

require 'rubygems'
require 'test/unit'
require 'dust'

unit_tests do
test "not negates true to false" do
assert_equal false, nil.not.nil?
end

test "not negates false to true" do
assert_equal true, Object.new.not.nil?
end
end

The implementation of the "not" method is similar to the implementation of the "as" method provided by facets (an explanation of the "as" implementation can be found in a previous entry).

class Object
define_method :not do
Not.new(self)
end

class Not
private *instance_methods.select { |m| m !~ /(^__|^\W|^binding$)/ }

def initialize(subject)
@subject = subject
end

def method_missing(sym, *args, &blk)
!@subject.send(sym,*args,&blk)
end
end
end

The Object.not method returns an instance of Object::Not. The Object::Not instance is initialized with the subject (the instance that was sent the "not" message). The Object::Not instances are basically proxies that send all calls back to the original instance. The Object::Not class privatizes almost all of it's methods so that most method calls will be handled by method_missing. The method_missing implementation simply forwards on any method call to the subject and negates the result.

27 comments:

  1. I'm a big fan of readability and I can see how this could be useful, but I'm not sure this is the best example. I'd probably re-write this code like this to make it more readable...

    if cart && cart.any? && cart.total > 0
    ...
    end

    Better still, Cart#total should probably be able to handle the case where Cart#empty? is true. Which means you could do this...

    if cart && cart.total > 0
    ...
    end

    You could also use the NullObject pattern to do something like this...

    cart ||= Cart.new
    if cart.total > 0
    ...
    end

    I'm sorry - I know this isn't the point you are trying to make and I know how hard it is to come up with good, simple examples - but I thought it was worth mentioning.

    ReplyDelete
  2. Excellent. Although I tend use 'unless' in this situation:

    unless cart.nil || cart.empty || cart.total == 0 do
    ..
    end


    Insightful post, though - it's helpful to see how this stuff can be implemented so easily in Ruby.

    ReplyDelete
  3. I'm wondering what that regex is doing - wouldn't it be easier to just grab all methods that end in ? instead?

    ReplyDelete
  4. Thanks for introducing me to the proxy object. I've got to say, tho, that method_missing is something you should really only use as a last resort. If you made heavy use of this & profiled your code, I'll bet you'd find your code spending half it's time in method_missing/send stuff.

    And isn't this more in keeping with a chain of method calls?

    cart.is_empty?.not

    All it would take is to add a not method to TrueClass & FalseClass.

    ReplyDelete
  5. Anonymous3:59 PM

    I like it. Someone get this into edge rails.. pronto!

    ReplyDelete
  6. I actually don't like cart.not.nil?. Just using cart seems good to me. However cart.not.empty? is pure gold.

    if cart && cart.not.empty?
    ...
    end


    Thats a nice little piece of code

    ReplyDelete
  7. Anonymous4:58 PM

    James,
    Fair enough. The example was contrived, and thus lost some of it's value I'm sure. Although, I do prefer obj.not.nil? to just obj. I think it's more expressive. But, I recognize that's a personal preference, which also goes against mainstream ruby.

    Jeff,
    You can definitely write the conditional statement in the way that you described; however, I prefer the version from my example since it's easier to read. I love the unless keyword when it's used following a statement; however, I dislike it when it begins a block of code. I find it harder to mentally parse. Perhaps I'm just slow. ;)

    Erik,
    "premature optimization is the root of all evil". --Donald Knuth
    While your statement that method_missing and send are expensive is correct , I expect most of my applications to have bottlenecks at the database or web service calls. I tend to write code that is easier to maintain (if I can) and only settle for a lesser solution when I profile and determine it's necessary. I think cart.not.empty? is much more readable than cart.empty?.not. In fact, in that case, I'd probably just stick with !cart.empty?

    Cheers, Jay

    ReplyDelete
  8. Anonymous5:03 PM

    Carlos,

    There's a lot of ways you can handle hiding most methods of an object. I think the BlankSlate version is probably the most popular since it has been around awhile. I chose the version I used in the example just because I stole it from the as definition from facets.

    Cheers, Jay

    ReplyDelete
  9. Anonymous5:46 PM

    Really cool idea.

    ReplyDelete
  10. agreed on premature optimization, but a not call is something that you are going to be hitting a lot. As an experiment, I'd love to see the profiling of, say, a version of the rails codebase with all nots & ! converted to this system. method_missing is a bad solution to a lot of problems.

    ReplyDelete
  11. Anonymous7:30 PM

    What about Ruby's regular not?

    if not cart.nil? and not cart.empty? and not cart.total > 0

    Although I agree with the suggestion to use unless/or, really.

    ReplyDelete
  12. Anonymous9:52 AM

    Erik,
    I agree that method_missing is generally a bad solution. I'm working with a huge codebase (16 ruby devs working for ~8 months at this point) and there were less than a hundred. I'll let you know if they turn out to be a problem when we profile.

    Sideshow,
    It is possible to just use Ruby's not; however, it's not as readable.

    ReplyDelete
  13. I'm looking forward to seeing your results. I've been down on method_missing ever since I discovered that XML::Builder uses it even when you call xml.tag!, a technique that wastes a LOT of time and serves no purpose.

    ReplyDelete
  14. I'm new to Ruby and am really loving the things you do to improve readability.

    "if cart.not.nil?" would indeed be very nice. And while we're at it, why not something like...

    "if cart.is.empty?" or "if cart.is.nil?"

    ...as an alternate to...

    "if cart.nil?" or "if cart.empty?"

    ReplyDelete
  15. Anonymous4:17 PM

    joey's comment above has brought back my initial "surprises" or "gotcha's" about Ruby conventions... Java has the convention of prefixing "is" to boolean method names... why has Ruby dropped it? Isn't "if array.is_empty?" more readable than "if array.empty?"?

    ReplyDelete
  16. Why not optimize your code for programming readability and your specs for whoever needs to read them?

    I mean, it is nice that Ruby lets us become as syntactically diabetic as we choose, but I personally think it's an anti-pattern to introduce meaningless syntax to make our code more 'englishy'.

    See COBOL for a reference. :)

    ReplyDelete
  17. also, I have a feeling you just want to use this with question mark methods, so maybe something like

    class Object
    def method_missing(id, *a, &b)
    if id.to_s =~ /not_(.*\?)/
    !send($1)
    else
    super
    end
    end
    end

    Might be better? At the very least, it saves you the creation of a proxy object.

    >> "foo".not_empty?
    => true
    >> "".not_empty?
    => false

    ReplyDelete
  18. Anonymous1:21 AM

    Just to make sure you're aware (and I'm under the assumption that this was while working on a Rails app?), Rails has a #blank? method that checks for nil and empty. So your if could have been "!cart.blank? && cart.count > 0" or whatever... but I definitely like the .not syntax :-)

    ReplyDelete
  19. I'd prefer something like this:

    if cart.any?
    ...
    end

    ...where Cart#any? returns true if there are any items and total > 0. To handle the case where cart is nil, you can open the NilClass and add

    def any?
    false
    end

    This is basically what rails does with NilClass#blank? so that you can test an object that you expect to be a String (but which might be nil) without testing for nil everywhere.

    ReplyDelete
  20. Anonymous2:56 AM

    This is kinda stupid.

    Learn how to use the language's operators. Maybe there was a reason they chose a ! operator instead of a textual "not" operator.

    If there are too many ! operators in your statement, use boolean algebra to make it more readable.

    Alternatively, break your boolean expression into several lines - assign sub-expressions to variables.

    The hack into the Object class is cute, but I think this is little silly. Interpreting boolean expressions, and writing them clearly is a core competency of any developer.

    ReplyDelete
  21. Anonymous1:53 PM

    I think cart.not.nil? is way better than !cart.nil? However, I do like cart.not_nil? more. What would be the downsides to Gregory Brown's approach of method missing? Is the problem that too many things hack method_missing without proper aliasing?

    Thanks.

    ReplyDelete
  22. Anonymous2:08 PM

    I try to stay away from method_missing in general since it's hard to debug and track down bugs involving it.

    When I do use method_missing, I tend to create objects that have no other responsibility. I find that doing this creates less chances for strange bugs. But, that's just my experience.

    And, there's always the concern of incorrect aliasing that you mention.

    ReplyDelete
  23. Anonymous8:07 PM

    An alternative to using method_missing could be to just define a not_x? method for each method x?. For new methods:

    class Object
    def self.method_added(name)
    return if name.to_s !~ /\?$/ || name.to_s =~ /^not_/
    define_method :"not_#{name}" do
    !send(name)
    end
    end
    end

    Also, negations for existing methods (like nil?) can be added based on similar criteria, giving you:

    class Object
    instance_methods.grep(/\?$/).each do |name|
    define_method :"not_#{name}" do
    !send(name)
    end
    end
    end

    The good thing is you can build a simple rule for patterns like lease.should_expire? so that your negation method is lease.should_not_expire? instead of lease.not.should_expire?

    However, it does mean adding a chunk of (sometimes unnecessary) methods to your objects.

    ReplyDelete
  24. Anonymous8:30 PM

    Farooq,
    I like it. You could also define methods on the Not instance dynamically.

    http://pastie.textmate.org/94011

    I'm not sure which is better though, truthfully.

    It would be interesting to find out.

    ReplyDelete
  25. Anonymous10:32 AM

    Jay this is ridiculous.

    Now if I see this code I say, "ok Array must understand #not...nope...ok maybe its in Enumerable...no...alright Array is a subclass of Object...oh there it is". That was a waste of my time.

    All that for very, very little readability. The ! is pretty much universally recognized as meaning not in most programming languages, therefore most developers are going to have no trouble reading it.

    As for #nil?, Object should have #not_nil? and that's it. If you need ! #empty?, write ! #empty? or #not_empty? (which is pushing it already).

    ReplyDelete
  26. Wow, what a firestorm this one caused. I'm writing this in October, though, and I have no idea how old the firestorm is, cuz the blog only shows comment times, not dates. Boo blogger.

    Anyway - I know this is a contrived example, but it does seem that this really only increases readability in the case where you have multiple boolean clauses with both positive and negative senses (because otherwise you could use unless/or). And nearly every time I see that happening, it's a code smell to me; I almost always want to factor it out into

    if response.ready_to_process?
    ...
    end

    def ready_to_process?
    return false if response.incomplete? || response.invalid?

    return false if response.total <= 0

    true
    end

    Because where there's three clauses today, you know there will be four tomorrow, and three's already too many anyway.

    ReplyDelete
  27. Giovanni.10:04 AM

    Thanks.

    ReplyDelete

Note: Only a member of this blog may post a comment.