Tracks: Part 3
This is the third of four parts in our series reviewing Tracks.
The last thing I wanted to comment on was Tracks’ use of Inheritance (a.k.a. STI). Rails lets you easily map an inheritance hierarchy to a single table using Single Table Inheritance. All you need to do is create a column called type and rails will take care of everything for you. An unfortunate side effect of making inheritance so easy, is that people use it for a variety of situations where it doesn’t make sense. Ask any of my Rails Core colleagues and they’ll tell you that STI abuse is my #1 Pet Peeve.
So lets take a look at the current tracks code.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
class Todo < ActiveRecord::Base # ... end class Immediate < Todo end class Deferred < Todo validates_presence_of :show_from def validate if show_from != nil && show_from < Date.today() errors.add("Show From", "must be a date in the future.") end end end |
From my understanding of the application, the intention is that a user can take a Todo and ‘defer’ it until some date in the future. At that point, the Todo should start showing up on the main lists again. To achieve this, tracks uses Inheritance with two subclasses of Todo:
- Deferred – For tasks which have been shelved until some future date
- Immediate – For tasks which should be done immediately
The main problem with using inheritance for this is that the you can’t, without serious hackery, change the class of an object, and Todo objects can be deferred at any time. Consider these two snippets; one of them should look completely wrong to you.
1 2 3 4 5 6 7 8 |
# clearly an abomination, which is why it's impossible in ruby. an_integer = 12 an_integer.class=String a_string = an_integer # this is what you'd expect an_integer = 12 a_string = an_integer.to_s |
If you find yourself manually updating the type attribute of an ActiveRecord model, you should step back, and reconsider your design: there’s no reason you should ever need to do that. Things in real life don’t magically transmogrify themselves, your model objects shouldn’t either.
A simple alternative is to add a deferred_until attribute to the the Todo table. If the deferred_until date is null, or in the past, a Todo is active, and if it’s in the future it’s deferred.
1 2 3 4 5 6 7 8 9 |
class Todo < ActiveRecord::Base def deferred? deferred_until && deferred_until > Time.now end def active !deferred? end end |
That gives you the ability to detect whether a Todo is deferred or active, but nine times out of ten you’ll want to split the Todos into two seperate lists. A naive implementation could consist of something like:
@active_todos, @deferred_todos = @project.todos.partition {|t| t.active? } |
But that’s really wasteful if you’re just looking for the active Todos. There are a few options to choose between. The easiest is to simply declare multiple associations. This gives you all the lazy-loading goodness which comes with an association, and accessing it is simple and unsurprising.
1 2 3 4 5 6 7 |
class Project < ActiveRecord::Base has_many :todos has_many :active_todos, :class_name=>"Todo", :conditions=>"deferred_until is null or deferred_until < NOW()" has_many :deferred_todos, :class_name=>"Todo", :conditions=>"deferred_until > NOW() " end |
:utc, and use Time.now.utc instead of Time.now when storing times. This simplifies TZ support in your app, and avoids issues when your server is in a different zone.
As nice as that option is, it suffers from two drawbacks.
First, it’s not database agnostic (the “NOW” function is named differently in just about every DBMS). This may not matter for most projects, but it can be a deal-breaker for a downloadable application like Tracks. If you don’t have control over the database that your users will want to configure with your application, you’ll need to avoid using database-specific features.
The second drawback: it’s ugly as hell. The Project and Context classes contain the logic about deferring tasks, when it should be nicely encapsulated in the Todo class. Thankfully, class methods on associations can help us here.
We need to define two class methods on Todo that handle the searching:
1 2 3 4 5 6 7 8 9 10 |
class Todo < ActiveRecord::Base #... def self.deferred find(:all, :conditions=>["deferred_until > ?", Time.now]) end def self.active find(:all, :conditions=>["deferred_until is null or deferred_until < ?", Time.now]) end end |
Now, everyone knows that this lets us call Todo.deferred to get a list of all deferred Todos across the whole system, but that’s not really that useful. What we really want is to find all the Todos for a given project or context, which match the other conditions. What some readers may not know is that ActiveRecord lets you do this:
@project.todos.deferred # ensures the Todo is deferred *and* belongs to the project |
That leaves you with what you’re after: you can get deferred todos, active ones, and also get access to all the Todos by calling @project.todos. The logic is nicely encapsulated in the Todo class, and it works across all supported databases.
Update: Tracks has already updated their design to remove inheritance from their Todo classes, the design is slightly different to what I proposed here, but still a nice improvement.
Just wanted to say thanks for all the hard work you guys are putting into the Rails Way. It’s so useful, and excellently written.
No worries Andrew, it’s heaps of fun to write too.
That’s an interesting point about how ActiveRecord will automatically make the deferred call in the context of @project.todos. It seems that Array is extended to only store objects of the given class (in this case Todo’s) and to act as a proxy for the association. While this is an excellent feature, I wonder why the Array class itself was extended as opposed to an AssociationProxy subclass of Array being created. It’s a bit misleading to have a collection of items (of Todo items) of class Array and then have an error raised when you try to add an item of a different (non Todo item) class. The only reason I can think of is because the [] language construct is hardcoded to create arrays, but that doesn’t seem like a good reason in this case.
Shalev, I’m not sure I understand your concern. ActiveRecord does use an association proxy, and the Array class is not modified in the least (by AR). If you do “x = foo.bars”, x is not set to an array, it is set to the association proxy that represents the bars collection. You have to explicitly do “foo.bars.to_a” to get the underlying array. (The proxy does quack like an array, though, so you rarely have to worry about the difference between the proxy and the underlying array.)
Assuming data models corresponding to the ones in the article:
I know that it is using an AssociationProxy (the callstack from that error reveals as much) – but todos still reports itself as an Array. That can be a bit misleading and I don’t understand why that is occuring in the first place. I know that using kind_of? is discouraged in favor of respond_to?, but even duck typing can be misleading as (for instance) the << operator restricts itself to objects of type Todo.
On a completely different note, the call @project.todos.deferred generatets to separate queries to the db. Would it be possible to have the AssociationProxy defer querying the db until strictly necessary? In such a case the call @project.todos would create the proxy but not actually query the db. Only when some sort of accessor method (.each, .[], etc.) is called would it generate the query. That would allow for the deferred options to be merged into the proxy before the db is actually queried.
Shalev, if you look at the source code for the AssociationProxy class in ActiveRecord (in lib/active_record/associations/association_proxy.rb), you’ll see near the top where it undefines a bunch of methods. The net result of this is that the method #class gets delegated to the target, so that “todos.class” will return the class of the underlying array, which is Array. You can show that what you have is a proxy by doing something like “todos.respond_to?(:proxy_target)”. It will return true if it is an instance of AssociationProxy.
But this is all part of the black magic of ActiveRecord. You should rarely need to know whether what you’re dealing with is an array or a proxy. If you are ever in doubt, just call “todos.to_a” and be done with it. :)
Shalev, also, regarding the multiple queries to the database, I’m not seeing that. @project.todos does not load the todos until you actually invoke something that the proxy has to delegate to the target. @project.todos.deferred will not actually load the target, it simply sees that the class of the association responds to #deferred and delegates to the class (with the appropriate scope set).
This series is really great. One question, though… what are some good uses of STI? There seems to be kind of a gray area for what are proper and improper uses of STI. I’d love to know what types of situations would be appropriate.
Sweet article Koz. An epitome of the Rails way.
As you didn’t show it, I’m assuming that Tracks originally had some code where they were explicitly changing the class of a Todo instance, is that correct? Whilst your solution is undoubtedly cleaner I’m a bit puzzled as to why they had to do this in the original code.
I’m a Java programmer learning Ruby (and Rails!) In Java we could declare a variable with the superclass type (Todo) and then assign an instance of the subclass type (Deferred) to it. Bog-standard OO stuff. My (limited) understanding of Ruby’s duck typing is that is that if you treat a Todo instance as a Deferred instance then it will act like one. Or does the interpreter complain unless you give it a hand and tell it that you want to use a Deferred instance by using the class-changing hackery?
John,
That’s right. This happened when a Deferred item’s time came about.
There was a second subclass of Todo called Immediate. We were changing a Todo from Immediate to Deferred. That was icky. Easier to do in Ruby than Java but a smell in either.
Ahh! So it was a sideways move in the inheritance hierarchy. Thanks for clearing that up, Luke.
If you find yourself manually updating the type attribute of an ActiveRecord model, you should step back, and reconsider your design
I have a property class with STI for home, flat, land, commercial, farm whatever. I can create a property object and change manually the type instead of doing the same thing over and over for each category. Is that so bad?
Erwann, if a piece of property can change from “commercial” to “farm”, then it is very likely that STI is the wrong way to solve the problem. Instead, you probably just have some attribute of the property that says whether it is commercial or farm or whatever.
Can someone explain where (other than in this excellent article) the association proxy magic is documented? “Some readers may not know”? How would any readers know?
I have an STI usage example where I need to change the class type, but even after reading the content above it still seems like a good use of STI. What I have is a CMS type thing for building forms. So the top level class is Element. Instruction, Heading, and Question all inherit from Element. For Question you can have Textfield, Textarea, etc. So you have something like Textfield < Question < Element.
Textfields and Textareas are handled differently, and inheritance seems appropriate, but what if a user changes their mind and wants to change a textfield to a textarea to allow longer answers?
Josh,
It’s behaviour that’ll drive the design. So without knowing what methods you have on the subclasses it’s hard for me to know for sure. but an alternative design would be:
class Question < ActiveRecord::Base belongs_to :question_type end class QuestionType < ActiveRecord::Base endThen if you want to change a question from a ‘text area’ to a ‘text field’ or vice versa, you can just do
Rather than having to transmogrify or hax anything :).
I agree with Tom, where is that documented. I know I’ve seen it said that you can do this:
@project.todos.find(...) # find in context of a projectBut I guess I didn’t know that class methods from Todo would automatically be available via the proxy. Does the proxy just grab all class methods and make them available, or does it actually try to only grab finders?
Also, if there aren’t docs for that, should the patch here: go on http://ar.rubyonrails.com/classes/ActiveRecord/Associations/ClassMethods.html ?
Rob, the
project.todos.find(...)thing is just a special case of the more general feature. Any class method on the association’s class can be accessed via the association. It’s just amethod_missinghack in the proxy.It is unfortunate that the association proxy classes are all
nodoc‘d. That would be the “right” place to put this. In the meantime, though, please do patch up the association docs to make that clearer, thanks!Another super useful article. In the projects I’ve worked on so far (newbie stuff), I’ve yet to find a reason to use STI, and I just used methods along the lines of what you’ve suggested. However, I didn’t realize you could do something like @project.todos.deffered—saves having to pass @project.id to the method to refine the search (which is what I’ve done). So this is a great help. Like Toby mentioned earlier, it would be nice to have a test case (perhaps in another class) to show when STI is useful – because it seems to me that most cases would be better solved as you demonstrated above? Obviously I must not be seeing the benefits of STI. Thanks, again. I’m learning a lot from these. The Ruby/Rails books (and other stuff online) that I’ve read focus a lot on the how, but not the why – which is a handicap, because unless you’re really smart (which I am not), a lack of understanding of the why really hinders adaptation and implementation.
zerohalo,
Unfortunately when people are taught object oriented programming, it’s usually with some contrived example of Triangle extending Shape. So inheritance becomes synonymous with OO. The reality is that Object Orientation is about making your application code match your underlying business, inheritance is a useful tool for a subset of cases, but by no means critical.
If we come across a submission which needs it, we’ll definitely cover the useful side of inheritance, but the reality is that most uses are actually abuses. I’ve always found Peter Coad’s rules on inheritance useful, but sadly
(submit button hit too eagerly)
Sadly most of peter coad’s stuff has fallen into obscurity as his books went out of print and he left IT after selling TogetherSoft to borland.
some readers may not know is that ActiveRecord lets you do this:
@project.todos.deferred # ensures the Todo is deferred and belongs to the pro[/code]
How does this work exactly? How does it link to the method deferred defined above? (does it even link to this?). Where is this documented? I couldn’t find anything in the online doc of ActiveRecord, ActiveRecord::Associations.
Regards