Hi folks! I'm about five weeks into Rails; this is my first question after lurking for a little while. I'm writing an app to help a printer verify that the information on magazine covers is correct; this verification happens across several organizations, and for several magazine titles. When a particular cover is "stalled" in one status for two long, the app needs to figure out who should get a warning message. The way the db is designed right now is: table "users" contains the users of the system; their logins, emails, etc. table "titles" contains magazine titles ("Celebrity Spoon Collectors") table "covers" contains individual covers ("Issue 2006.12.A3") table "notifications" describes which users get warnings for a title, and which users get notified when all verifications are complete. It contains user IDs, title IDs, and booleans for "warning" and "verification." table "users" has_many :notifications table "titles" has_many :covers and has_many :notifications table "covers" belongs_to :title table "notifications" belongs_to :title and belongs_to :user So when the question arises: "Given an overdue cover, who should get a notification about it?" I *could* do this: for each user in cover.title.notifications [send email, etc...] but "reaching" through three tables like that just seems kludge-y. From "cover", travel up to that cover's title, then over to the notifications for that title, then get the user from each matched object, etc... Can you kind souls tell me if there's a "preferred" way to handle this? * No, dot-script notation through three tables is fine? * Write a custom find method in model "notifications" so that you can just ask that model for notifications, rather than doing things the hard way in the controller? Class Notification def self.find_users_to_notify(cover, event) [big ol' "find_by_sql" left join find goes here] end end Then my code could be like this: for each user in Notification.find_users_to_notify( cover, "warning" ) * Something else that will cause me to slap my forehead? Thanks for any advice! Right now, I'm using dot-script, but it seems... painful. Best regards, John Young
on 2006-02-04 05:37
on 2006-02-04 07:15
On Feb 3, 2006, at 7:37 PM, John Young wrote: > I *could* do this: > for each user in cover.title.notifications > [send email, etc...] > > but "reaching" through three tables like that just seems kludge-y. > From > "cover", travel up to that cover's title, then over to the > notifications > for that title, then get the user from each matched object, etc... Do *not* prematurely optimize. Do you have a performance problem? Will you need to send millions of emails per day? Of course not. Do it the way that is easiest to understand for anyone looking at the code. You, and they, will be happy that you did. -- -- Tom M.
on 2006-02-04 13:34
> Do *not* prematurely optimize. > > Do you have a performance problem? > Actually, I wasn't thinking of this as an optimization issue so much as an MVC philosophy question. Just as it seems to be a best practice to keep the object logic in the model, and the business logic in the controller, I was wondering if there was a corresponding best practice when asking an object about its cousin's best friend's wife's high school buddy's attribute :) So maybe the decision between these things: ...for user in cover.title.notifications.users... ...for user in Notifications.find_users_to_notify_for(cover)... ...is a matter of deciding what's easiest to understand? The first is explicit, and the second hides the actual logic of the find in the Notification class. Would love to know if there's consensus on a Ruby idiomatic way of doing things. Like, for instance, I hear it's idiomatic to say cover.title rather than cover[:title]
on 2006-02-04 14:37
John Young wrote: > > So maybe the decision between these things: > > ...for user in cover.title.notifications.users... > ...for user in Notifications.find_users_to_notify_for(cover)... I think this looks best: Notifications.find_users_to_notify_for(cover).each do |u| #logic end If you need this in several places and something in the logic changes, you will have to go through and change everything. Whereas with the second method, you only need to change one place. Hope this helps Joey
on 2006-02-04 22:14
On Feb 4, 2006, at 3:34 AM, John Young wrote: > when asking an object about its cousin's best friend's wife's high > school buddy's attribute :) > > So maybe the decision between these things: > > ...for user in cover.title.notifications.users... > ...for user in Notifications.find_users_to_notify_for(cover)... > > ...is a matter of deciding what's easiest to understand? The first is > explicit, and the second hides the actual logic of the find in the > Notification class. Ah, sorry for the misunderstanding. Yes, by all means, take the second option! Quite a lot more clear what is happening in real terms. -- -- Tom M.
on 2006-02-04 22:45
Just a little quibble on terminology... "Object logic" doesn't really mean anything... other than maybe the underlying logic in the language for dealing with objects. "Business Logic" generally refers to the rules pertaining to the problem domain and should therefore be in the model. That's more MVC. I think by "Business Logic" you were refering to "Application Logic", which is the functionality provided by a given application. This should be contained in the controllers and one should be careful not to let it intermingle with business logic. You can think of application logic as the stuff that dictates the flow of pages or how we get information to/from the model, whereas the model captures the essence of the problem domain; one should be able to detach it from the application and plug it into a different application. I will agree, however, that one's skin should be crawling when using chained method calls. This is considered a no-no by OOP purists; look into the Law of Demeter to see what I mean. Now, the problem with this is that you wind up writing lots and lots of delegate methods... gets pretty verbose to keep this up (I often cheat). And ActiveRecord makes this all too easy, since hooking up the objects is done declaratively. That makes adding the find method good object dependecy practice as well as more explicit. b
on 2006-02-07 19:24
Ben, Tom, and Joey, thanks very much! This is reassuring that I'm not entirely on the wrong track. I'm writing some delegate methods now, and leaving some other short chains alone, using ease of understanding as my goal. Ben M. wrote: > ...I think by "Business Logic" you were refering to "Application Logic",... > ...look into the Law of Demeter ...