Jason touched on it before, but there are a few reasons for the way that
Page#part(name) works. If I remember correctly, it originally worked as
Quake Wang describes, that is, just the 'else' part of the 'if'
statement. However, consider the situation where you have added a part
but not saved the Page. What do you do when the page has existing parts
that _have_ been saved? Obviously, if you were trying to find again the
part that you just added, it would not be found. Also, what if the
existing parts haven't been loaded yet from the database? The first
branch of the 'if' clause handles those situations.
Now, if I remember correctly, the actual use-cases that inspired this
change were the share_layouts and preview extensions. When rendering a
controller action, share_layouts creates a Page in memory, populates its
parts with the output of the template, then renders the page without
saving it. The preview extension does something similar, but simply
renders the page data that was posted to it.
Sean
Jason Garber wrote:
Here's the code you couldn't find in 0.8.0:
http://github.com/radiant/radiant/blob/918ff4e00c0f94192fa92c551358d5365284472a/app/models/page.rb#L71
def part(name)
if new_record? or parts.to_a.any?(&:new_record?)
parts.to_a.find {|p| p.name == name.to_s }
else
parts.find_by_name name.to_s
end
end
Your simplification fails the "Page should allow access to parts by
name created with the build method when page is unsaved" spec. I
wouldn't recommend leaving it that way in your app.
Jason
On Jun 30, 2009, at 8:15 AM, Quake Wang wrote:
Thanks for your detailed explanation, however, I cannot find any
new_record checking usage in 0.8.0 code base. In other words, my
application is still working fine after simplify the code:
def part(name)
parts.find_by_name name.to_s
end
Regards,
Quake
On Tue, Jun 30, 2009 at 4:19 PM, Jason Garber<[email protected]> wrote:
Having just spent a few hours in those methods (upgrading Chronicle to
0.8.0), I can speak to your question as it relates to 0.8.0.
Page#parts is
an ActiveRecord association: it loads all the PageParts that belong
to that
page into an array and then just uses that array from then on unless
you do
a find or a count, at which point it goes back to the database to
respond to
your find/count. This is important because if you create a new part
(through parts.build or parts_attributes=), it just gets added to
that array
and ultimately saved to the database when you save the page. Thus,
if the
page and all its parts are not new records, the instantiated page/parts
should match what's in the database and a SQL find should be faster
than an
Array find. If there were any new records, though, they would not
be in the
database, so your part(name) would miss them.
Your question raises an interesting point, though: If we're already
doing an
any? on the parts, why would the SQL query be any faster than
Array#find? I
did a little benchmarking...
http://gist.github.com/138062 (run with -f profile)
SQLite3:
10.3987880 Page#part(name) on a large set of parts using SQL find
6.5138080 Page#part(name) on a small set of parts using SQL find
4.0115610 Page#part(name) on a large set of parts using array find
0.1251080 Page#part(name) on a small set of parts using array find
MySQL (on a slower machine):
32.3944890 Page#part(name) on a large set of parts using SQL find
17.4200660 Page#part(name) on a small set of parts using SQL find
13.0136880 Page#part(name) on a large set of parts using array find
0.5342530 Page#part(name) on a small set of parts using array find
Sean, does what I'm measuring make sense? Any reason we shouldn't just
switch completely to Array#find for the Page#part method?
Jason
On Jun 29, 2009, at 5:04 AM, Quake Wang wrote:
Hi,
I found that the "part(name)" method of Page model generating many
sql query in development env, I'm curious to know why we need to check
new_record for Page and its parts?
Regards,
Quake
_______________________________________________
Radiant mailing list
Post: [email protected]
Search: http://radiantcms.org/mailing-list/search/
Site: http://lists.radiantcms.org/mailman/listinfo/radiant
_______________________________________________
Radiant mailing list
Post: [email protected]
Search: http://radiantcms.org/mailing-list/search/
Site: http://lists.radiantcms.org/mailman/listinfo/radiant
_______________________________________________
Radiant mailing list
Post: [email protected]
Search: http://radiantcms.org/mailing-list/search/
Site: http://lists.radiantcms.org/mailman/listinfo/radiant
_______________________________________________
Radiant mailing list
Post: [email protected]
Search: http://radiantcms.org/mailing-list/search/
Site: http://lists.radiantcms.org/mailman/listinfo/radiant
_______________________________________________
Radiant mailing list
Post: [email protected]
Search: http://radiantcms.org/mailing-list/search/
Site: http://lists.radiantcms.org/mailman/listinfo/radiant
