Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 12 years ago

#3806 closed enhancement (fixed)

Deprecate sprintf() in 'alt' param of bp_core_fetch_avatar()

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: 1.6 Priority: normal
Severity: normal Version:
Component: Core Keywords:
Cc:

Description

bp_core_fetch_avatar() takes, as an 'alt' param, a string that is usually passed with arguments meant to be swapped out using sprintf() with some object data (group name, member display name, etc). The problem is that, in order to get this data, we have to call up group or member objects, which means a ton of extra queries (for data that is, moreover, almost always already available in an existing global).

Can we deprecate the feature of bp_core_fetch_avatar() that allows/encourages folks to pass these kinds of strings? Instead, we should pass fully concatenated strings, and encourage plugins to do the same, using the data they already have available.

We could maintain backpat by sniffing out '%s' in the alt param, if anyone thought it mattered that much.

I'll post a patch shortly with my suggestion. Would be very easy to do for BP 1.6, and would give *huge* performance boosts in many situations - dozens fewer queries per page load, on directories.

Attachments (1)

3806.01.patch (7.2 KB) - added by boonebgorges 12 years ago.

Download all attachments as: .zip

Change History (17)

#1 @boonebgorges
12 years ago

This patch is getting tiresome so I'm going to post a half-done version of it. I want to make sure that no one has a problem with this before devoting the energy to finishing it.

#2 @boonebgorges
12 years ago

(In [5466]) Deemphasizes sprintf() type 'alt' parameters for bp_core_fetch_avatar(), in order to avoid unnecessary database queries when loading avatars. Modifies calls throughout bp_core_fetch_avatar() so that they pass literal strings as 'alt' parameters. See #3806

#3 @boonebgorges
12 years ago

  • Keywords dev-feedback removed

r5466 takes care of much of this ticket. It changes the logic in bp_core_fetch_avatar(), and it changes all calls to bp_core_fetch_avatar() throughout the members component so that full literals are passed as 'alt'. I'll work my way through the rest of the components over the next few days, though this commit gives us the bulk of the benefit.

#4 @johnjamesjacoby
12 years ago

(In [5468]) Remove debug cruft from r5466.

In bp_core_fetch_avatar():

  • Use switch statements to set defaults
  • Use empty() to prevent notices
  • General code clean-up
  • See #3806

#5 @johnjamesjacoby
12 years ago

(In [5470]) Revert part of r5466. Directly referencing numeric array keys is naughty; use stdClass and vars instead. See #3806.

#6 follow-up: @boonebgorges
12 years ago

Remove debug cruft from r5466.

Whoops, sorry about that.

Use empty() to prevent notices

We set defaults early in the functions, so this typically isn't needed (though it doesn't hurt)

Revert part of r5466.

You mean r5467. Scold thyself :)

#7 @boonebgorges
12 years ago

(In [5477]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout activity component. References #3806

#8 @boonebgorges
12 years ago

(In [5478]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout blogs component. References #3806

#9 @boonebgorges
12 years ago

(In [5479]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout core component. References #3806

#10 @boonebgorges
12 years ago

(In [5480]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout forums component. References #3806

#11 @boonebgorges
12 years ago

(In [5482]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout friends component. References #3806

#12 in reply to: ↑ 6 @johnjamesjacoby
12 years ago

Replying to boonebgorges:

You mean r5467. Scold thyself :)

I was hoping you wouldn't notice. :)

#13 @boonebgorges
12 years ago

(In [5500]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout groups component. References #3806

#14 @boonebgorges
12 years ago

(In [5501]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout messages component. References #3806

#15 @boonebgorges
12 years ago

(In [5503]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout bp-default. References #3806

#16 @boonebgorges
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [5504]) Pass concatenated alt parameter to bp_core_fetch_avatar() throughout xprofile component.
Fixes #3806

Note: See TracTickets for help on using tickets.