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).
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).
define_method :not do
Not.new(self)
end
private *instance_methods.select {|m| m !~ /(^__|^\W|^binding$)/ }
@subject = subject
end
!@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.
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...
ReplyDeleteif 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.
Excellent. Although I tend use 'unless' in this situation:
ReplyDeleteunless 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.
I'm wondering what that regex is doing - wouldn't it be easier to just grab all methods that end in ? instead?
ReplyDeleteThanks 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.
ReplyDeleteAnd 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.
I like it. Someone get this into edge rails.. pronto!
ReplyDeleteI actually don't like cart.not.nil?. Just using cart seems good to me. However cart.not.empty? is pure gold.
ReplyDeleteif cart && cart.not.empty?
...
end
Thats a nice little piece of code
James,
ReplyDeleteFair 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
Carlos,
ReplyDeleteThere'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
Really cool idea.
ReplyDeleteagreed 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.
ReplyDeleteWhat about Ruby's regular not?
ReplyDeleteif not cart.nil? and not cart.empty? and not cart.total > 0
Although I agree with the suggestion to use unless/or, really.
Erik,
ReplyDeleteI 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.
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.
ReplyDeleteI'm new to Ruby and am really loving the things you do to improve readability.
ReplyDelete"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?"
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?"?
ReplyDeleteWhy not optimize your code for programming readability and your specs for whoever needs to read them?
ReplyDeleteI 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. :)
also, I have a feeling you just want to use this with question mark methods, so maybe something like
ReplyDeleteclass 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
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 :-)
ReplyDeleteI'd prefer something like this:
ReplyDeleteif 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.
This is kinda stupid.
ReplyDeleteLearn 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.
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?
ReplyDeleteThanks.
I try to stay away from method_missing in general since it's hard to debug and track down bugs involving it.
ReplyDeleteWhen 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.
An alternative to using method_missing could be to just define a not_x? method for each method x?. For new methods:
ReplyDeleteclass 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.
Farooq,
ReplyDeleteI 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.
Jay this is ridiculous.
ReplyDeleteNow 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).
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.
ReplyDeleteAnyway - 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.
Thanks.
ReplyDelete