Wednesday, June 28

DHH's Kool-Aid is Mighty Tasty!

As I suspected, I didn't have all the information. That's why I like to cover my ass with statements like:

"I hope that I'm wrong here and some noble Rails advocate will set me straight..."

And as luck would have it, The Man himself set me straight (or somebody doing a convincing job of impersonating him... where's OpenID when you need it?):

The "problem" with the code you show seems to be a misunderstanding of how associations in Rails work. A call like collector.possessions.delete(possession) is not MEANT to destroy the possession. You do that by calling either possession.destroy or collector.possessions[1].destroy. The call you're making reads like this "delete the possession from the collector's list of possessions (thus making you free to give the possession to someone else)". Considering that the method is named delete, though, I'll treat it as an honest mistake...

Hmm, that's not exactly what I would consider an intuitive API, but I'll give it a test run. First, let's see if I can really "destroy" that record. The code:
collector = Collector.find(19) 
possession = collector.possessions.find_by_collectible_id(72)
possession.destroy

And the query:
mysql> select * from possessions;
Empty set (0.00 sec)

Yup, it's gone! Score one for my misinterpretation of the API.

Now let's see about that giving the possession to someone else. The code:
collector = Collector.find(19) 
possession = collector.possessions.find_by_collectible_id(72)
collector.possessions.delete( possession )
anotherCollector = Collector.find(33)
anotherCollector.possessions << possession

(Yes, I recreated the record after the prior test -- I didn't just pass a nil from one collection to another) And the query:
mysql> select * from possessions;
+----+--------------+----------------+----------+
| id | collector_id | collectible_id | quantity |
+----+--------------+----------------+----------+
| 3 | 33 | 72 | NULL |
+----+--------------+----------------+----------+
1 row in set (0.00 sec)

Sure enough, it works just like David said. Who would have figured he'd know? ;-)

Now that I know I have to "destroy" my relationships instead of simply "deleting" them from their respective collections, I can accomplish what I'd hoped from my last rant, that being a clean database. However, I don't think this is very intuitive and I think it's a rather gaping hole in the design that when I do "delete" a relationship from a collection it becomes an orphaned record until such time as I "append" it to another collection, and if that time never comes (as it didn't in the example from my last post) I've got litter in my database. That's not a Good Thing(tm), and it could be prevented at the database level with foreign and/or composite keys if Rails' behavior wasn't to nullify one off the references. But, since that's the way Rails acts, I can't defy it at the database level without causing it to break or code around it, both of which completely deter from the beauty, simplicity, and speedy development we've come to expect from the framework.

So, in conclusion, it is workable, but it's not elegant. And, in my humble opinion, it's not intuitive and it's not The Right Way(tm). But there are no silver bullets or golden hammers out there, and when it comes to weighing pros to cons this con isn't nearly big enough (now that I know the work-around) to out-weigh all the pros.

2 comments:

DHH said...

I'm sorry, but I really don't follow where the evil comes in. The only possible API optimization I can see in this example is perhaps calling it collector.possessions.remove(possession) instead of delete to even further encourage the notion that you're removing an element from a collection, not deleting the element itself.

It helps if you stop thinking about Possessions as merely a relationship. If you start thinking of it as a model on equal parts with Collector and Collectibles, then it's a lot more reasonable to deal with it just like you would any other model.

Additionally, all of this is actually explained in the API docs. Under has_many.delete, you'll find this paragraph: "collection.delete(object, …) - removes one or more objects from the collection by setting their foreign keys to NULL. This will also destroy the objects if they’re declared as belongs_to and dependent on this model.". (see http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html).

I know that its unsexy to read docs and that you hopefully shouldn't have to most of the time. But whenever you're in doubt or think you've found a bug, it's always a good idea to check what the expected behavior of something is.

In any case, as long as we can reframe this discussion to be one of what's the nicest API for something, instead of getting sweating palms over Rails supposed lack of foreign key support, then it's all good.

Trak3r said...

> I'm sorry, but I really don't follow where the evil comes in.

I'm speaking tongue-in-cheek here, not meaning to imply any malice. I apologize if that's not coming across in my writing style. I have been accused in the past of being too "dry" (not be be confused with DRY ;-)

> It helps if you stop thinking about Possessions as merely a relationship. If you start thinking of it as a model...

Yes, that does help considerably. It seems that the Rails mindset still hasn't completely sunk into my noggin, despite the documentation and several dozen blogs I read on the topic every day :-)

> Additionally, all of this is actually explained in the API docs ... "... removes one or more objects from the collection by setting their foreign keys to NULL ..."

Yes, I saw that and quoted it in the comments for my plug-in (pasted in my last rant). I wasn't convinced that it was the correct behavior, but now that I'm seeing your point of view, I understand it.

> This will also destroy the objects if they’re declared as belongs_to and dependent...

This part confuses me. I read that as saying it will delete the objects being joined by the relationship, not just the relationship itself, and if that's correct, it's definitely not the behavior I desire (or expect).

> I know that its unsexy to read docs

Ha ha, touche! ;-) I assure you I did read the docs, I just didn't completely grok them, as is apparent by my rants. I've been writing for nearly a decade now and I learned the hard way so many years ago to make damn sure you know what you're talking about before you make yourself look like a jack-ass in print. With that in mind, please don't Google up any of my earlier publications ;-)

> But whenever you're in doubt ... always a good idea to check what the expected behavior of something is.

That's exactly what I did, and when I didn't see my expected behavior, I ranted about it :-)

I'm impressed and thankful that you took the time to address my confusion personally; I'd imagined you'd be far too busy to bother will my little blog. But as you can see from the posts of others, and even from the comments of some presenters at the recent conference, I'm not alone in my inability to see the design sense through your lenses. Even after reading Josh's great posts on all the ins and outs of has_many I still didn't completely get it. And now that I think I do get it, I'm not entirely convinced [yet] to agree with it. But I'm willing, and trying, to work with it rather than against it, as long as somebody [or some documentation] is able to explain to me when I think it's working against me and how I need to change my point of view :-)

Thanks David.