Posted by Doug
Fri, 24 Oct 2008 17:30:00 GMT
Frankly, I haven’t ever really made up my mind about how to address the Law of Demeter in Rails. I’d like to use this article as a spring board to discuss the alternatives.
As found on Wikipedia, the Law of Demeter says “Only talk to your immediate friends.” There’s a good write-up about it by Brian Donovan specifc to rails, Law of Demeter, or How to avoid coding yourself into a corner in Rails. In this article he sums up nicely the advantage of delegation.
Let’s look at an example:
class User < ActiveRecord::Base
has_one :profile
end
class Profile < ActiveRecord::Base
has_one :address
end
class Address < ActiveRecord::Base
end
Now if you want to find out what country the user lives in you need to access user.profile.address.country. That’s a violation of the LoD. The problem is if you refactor profile or address you’ll have to go back and fix all the places you did this access chaining as well. The common answer is to delegate!
class User < ActiveRecord::Base
has_one :profile
delegate :country, :to => :profile
end
class Profile < ActiveRecord::Base
has_one :address
delegate :country, :to => :address
end
class Address < ActiveRecord::Base
end
Now we can just access user.country. The problem so far is that (at least in Rails 2.1) if the user’s profile is nil or if the profile’s address is nil then the call to user.country will blow up. There are some patches and work arounds, but the problem exists. Also, this double delegation smells to me and I don’t know why. Maybe it’s the obtuseness of this particular design. Maybe double-delegation is a smell of bad modeling.
There’s a new feature in Rails 2.2 that relate to delegation and using a prefix for delegated methods. This might be a little contrived, but continuing with our above example:
class User < ActiveRecord::Base
has_one :profile
delegate :country, :to => :profile, :prefix => true
end
Means that now we call user.profile_country instead of user.country. Instead of passing true to the prefix option, you can also pass a string that will get used instead of the delegator object name.
I question why user.profile_country is any better than user.profile.country. If you rename the profile class, you’re almost always going to want to rename that prefix too. So everywhere in your code you called user.profile_country you’ll end up changing it to user.foobar_country.
Finally, I’m going to bring up Jay Field’s Presenter Pattern. Basically, this is creating an object that aggregates multiple objects so they can easily be presented on the same form simply. In addition, the presenter object gives you a place to put formatting and some computational logic that sometimes finds its way into views.
The Presenter Pattern seems neat, but I’m not sure when to use it instead of delegation straight from the “parent” object. In the example on Jay’s page he talks about a user_account, user_credential, and an address. All that gets aggregated into a CompletePresenter. But really, that seems contrived too. His user account only has a name and the credentials has the username and password. It seems more likely that you’d just define a user that has a name, username, and password and allow the user to has_one address. Why not just delegate the address attributes directly from the now combined user class?
I’ll also mention the Rails fields_for helper. This seems to be an effort to address having multiple objects on the same web form. Personally, I think the net effect of that is to push more logic into your controller to deal with these multiple groups of fields. Both the presenter pattern and straight up method delegation push this back into the models and out of the controllers. Maybe I just don’t understand how this is meant to be used.
So I’ll close with these questions. When is the Presenter Pattern better than delegation from your already existing models? When is the added complexity of an additional layer of objects worth it? What about double delegation? Is it a smell and for what? Should I just get over my unease when I see double delegation? What’s the real value of using a prefix for delegated methods? What are the legitimate uses of fields_for? Also, why doesn’t the Rails delegate method handle the case where the delegation target is nil? I know there are work arounds and patches, but it seems like the Rails implementation of delegate is broke.
Posted in Programming, Ruby on Rails | Tags delegate, demeter, design, pattern, presenter, Rails, Ruby | 2 comments
Posted by Doug
Tue, 05 Feb 2008 21:35:00 GMT
I’ve just fixed a bug in production that took me more than eight hours to find. When I show you the code, you’ll wonder why it took me so long. I have lots of excuses, but it’s a fairly interesting bug to think about. It shows some of the weaknesses in my usual modus operandi. The code that looks something like this:
class Article
CURRENT="start_publish_on <= #{Date.today} AND stop_publish_on > #{Date.today}"
def self.find_latest
find(:all, :conditions => CURRENT)
end
end
In retrospect the bug is obvious and probably is obvious to you as well. The Article::CURRENT constant is dynamically generated using the date at the time the class is evaluated. With rails in production mode, that could be a long time; certainly more than a day. The conclusion to draw here is to be very, very careful about dynamically generating constant strings. As a rule, I might suggest not doing it.
The most interesting thing about this is you can’t write a test to catch this error. I think that’s the the biggest thing that took me so long to find the error. I tend to be over confident in our test suite. As the new guy to the project, I’m proud of them for how conscientious they are about testing their code. I’m trying to fix this bug by triggering it in a test. Well, it can’t be done. The difference is how the production environment cache classes versus how testing and development does it.
Here’s another tidbit that threw me off the trail for a long time. Our copy editor that is responsible for publishing articles says to fix it she simply back dates the articles by a day. So I spent a lot of time looking for off-by-one errors. I had recently fixed a problem with comparing times to dates and causing off-by-one, so I thought that might be it. As it turns out, this was a red herring. There’s such a tight loop for feature request, implement, deploy that the production environment gets restarted fairly regularly (like nearly every day).
I guess what prompted me to write about this particular bug was what it said about our testing. Clearly automated testing can’t find all the bugs. It also says something about our rapid development. As long as we’re really busy, this bug didn’t bite us. It’s not until our deployment slows down (like a weekend) that it showed up.
Posted in Programming, Ruby on Rails | 1 comment
Posted by Doug
Wed, 07 Nov 2007 16:04:00 GMT
One of the things that’s really nice about TextMate is the Cmd-T navigation of files in your project. It pops up this little dialog with fancy pattern matching input to select one of the files in your project and then jumps to that file. There’s a similar command once your in a file to jump to symbols in that file. I’ve tried a couple things to achieve that behavior in emacs that I’d like to talk about.
The first thing I tried was a method called find-file-in-project. It was originally implemented by Phil Hagelberg as part of his “Another Ruby on Rails Mode” (arorem) and then also ported over to the current Rinari Is Not a Rails IDE. Basically, it indexes all the files in a “project” and then provides a nice interactive completing read to switch between them. It basically works just like Cmd-T in TextMat. I did quite a bit of work optimizing the completing read so that it would behave nicely. The problem is that emacs is slow to index the files in your project.
What I’m using now is the tried and true find-tag method which is part of etags.el. ETags depends on an external TAGS file as an index of all the symbols in your project. When you invoke find-tag (by default bound to M-.) it prompts you with completing read for the symbol to find. It then jumps directly to the file and location where that symbol is defined. It’s basically combining the find-file-in-project with find-symbol-in-buffer. It’s also very, very fast.
As an added bonus, you can use tags-search to search through your project finding places that tag is used. This is similar to TextMate’s slowly grep all the files and render a buffer with those results in it, but much faster.
The bad news is you have to manually build the TAGS file periodically. Emacs is pretty good about continuing to work when you’ve made modifications to the files, but if you add new methods, new files, or refactor significantly where thing are located then it gets confused. When that happens, simply rebuild the TAGS file.
Jim Weirich gave me a nice little rake task to do the job:
module Tags
RUBY_FILES = FileList['**/*.rb'].exclude("pkg")
end
namespace "tags" do
task :emacs => Tags::RUBY_FILES do
puts "Making Emacs TAGS file"
sh "xctags -e #{Tags::RUBY_FILES}", :verbose => false
end
end
task :tags => ["tags:emacs"]
Just put that in lib/tasks/tags.rake and then run rake tags:emacs when you invoke find-tag and it can’t find it. That shouldn’t happen very often. I’ve found it’s also very fast to build the TAGS file. I might consider putting that tags task as part of the normal run tests task, but I’m not sure that’s necessary.
The other catch here is that the rake task above calls out the exuberant ctags rather than the ctags that comes with emacs. The exuberant ctags knows how to parse ruby files whereas the ctags that comes with emacs can’t.
I’ve installed exuberant ctags from MacPorts. It is xctags even though MacPorts doesn’t install it that way. So (for better or worse) I’ve renamed the ctags files installed with ports to xctags. This also gets around the conflict that MacPorts thinks there is between the ctags installed with emacs and the exuberant ctags.
Give this a spin and let me know what you think. I’ve found it to be very accurate and very fast.
UPDATE: I’ve added a github repository for my find-file-in-project. Feel free to do with it as you will.
Posted in Emacs, Programming, Lisp, Ruby on Rails, Mac OS X | Tags emacs, etags, rinari, Ruby, TextMate, xctags | 4 comments
