@Lathi.net

Suggestion of Demeter

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

Scripting Screen

Posted by Doug Sat, 13 Sep 2008 14:11:00 GMT

I do pretty much all my work using GNU Screen inside a terminal. I use a different screen session for each project and have pretty much the same window configuration for each session. After finally getting tired of manually setting up my screen sessions, here’s how I managed to script new session setup.

Like many I have a development directory where all my projects live. Inside this devel dir I have the trunk of each project checked out from version control. Most of my projects haven’t been converted to git yet, so I’m not sure how different it’d be compared to svn. My screen session is made up of several windows. Inside the first, I run emacs. The second is autotest. The third I run ./script/server. I also have a window for ./script/console and a database prompt; but the order isn’t that important. After those I’ll have a window or two for misc. bash prompts.

One solution to automatically setting all this up is to put screen commands inside your .screenrc to create and name the windows. You can even specify what command to run in the windows. The problem is that when you exit that command for whatever reason, the screen window is killed with it. That’s not so handy when you need to restart autotest or your server. Also, the .screenrc file doesn’t allow for any type of scripting for dynamically generating any of your startup information.

After digging in the man page a little bit I found a few screen commands that allow it to be scripted fairly well from outside scripts. I’ll show the important commands here.

This will create a new screen session detached but with an input/output stream. The -S names the screen session to make it easier to refer to it later.

This sends a screen command to the screen session with the given name. In this case, the screen command is to create a new screen window named “test” as window 1.

screen -X -S <session name> -p 1 stuff "cd $DEVEL/$PROJECT; autotest
" 

This again sends a screen command to the named session. In this case we’re sending the stuff command directly to window 1. The stuff command sends the string into stdin of the running application in that window. In this case the running app is a bash prompt. Note the trailing newline inside the string will cause the command “typed” at the bash prompt to be run.

The only other problem I ran into is that when creating screen windows like this, they didn’t show up on my hard status line automatically. My solution was to setup a little for loop and select each window like this:

for i in 0 1 2 3 4
do
   screen -X -S <session name> select $i
done

Finally, when all is setup the way you want the last thing to do is join the resulting screen session like this:

The trailing -p 0 says to start in the first window.

You can download my resulting script here. One thing to note is that I have an entry in /etc/services for each of my projects. I use this so that when I run ./script/server I always start on the same port for each project. Also, I’m making use of bash’s select command for prompting with project (or which already started screen session) to join/start. It’s a little clumsy in that select wants to keep prompting you for input after you’re done with the screen session; but it’s a lot better than what I had.

Posted in Programming | Tags emacs, screen, script | 3 comments

Dynamic Constants and their Pitfalls

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