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<j...@jasongarber.com> 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:   Radiant@radiantcms.org
Search: http://radiantcms.org/mailing-list/search/
Site:   http://lists.radiantcms.org/mailman/listinfo/radiant

_______________________________________________
Radiant mailing list
Post:   Radiant@radiantcms.org
Search: http://radiantcms.org/mailing-list/search/
Site:   http://lists.radiantcms.org/mailman/listinfo/radiant

_______________________________________________
Radiant mailing list
Post:   Radiant@radiantcms.org
Search: http://radiantcms.org/mailing-list/search/
Site:   http://lists.radiantcms.org/mailman/listinfo/radiant

_______________________________________________
Radiant mailing list
Post:   Radiant@radiantcms.org
Search: http://radiantcms.org/mailing-list/search/
Site:   http://lists.radiantcms.org/mailman/listinfo/radiant


_______________________________________________
Radiant mailing list
Post:   Radiant@radiantcms.org
Search: http://radiantcms.org/mailing-list/search/
Site:   http://lists.radiantcms.org/mailman/listinfo/radiant

Reply via email to