field_multilingual_available_languages() does not use advanced drupal_static() pattern [#672480]

File	Size	Author
drupal_static_fields.patch	970 bytes	catch
drupal_static_before.png	104.6 KB	catch
drupal_static_after.png	57.37 KB	catch

Comment #1

yched commented 2 January 2010 at 13:14

Status:

Needs review

» Reviewed & tested by the community

Looks good to me. Should we add a comment on those 'advanced drupal_static() pattern', or are they considered standard already ?

Log in or register to post comments

Comment #2

catch

he/him

English

commented 2 January 2010 at 13:53

There's a big phpdoc hunk in drupal_static() which went in with the original patch, the hope is that people look there when they see the weirdness.

Log in or register to post comments

Comment #3

effulgentsia commented 5 January 2010 at 19:22

Just my $0.02, but I would really like to see #619666: Make performance-critical usage of drupal_static() grokkable land first, and this be re-rolled with that change. The other uses of the advanced pattern currently in HEAD do not pass a default to drupal_static(), and one reason I rolled the patch that introduced the advanced pattern that way is because that middle line is so long and obscure that I thought it would be better DX to have it always be identical everywhere it's used. However, I think the cleanup patch in that issue makes the line sufficiently more grokkable that having a use-case like this that passes a default to drupal_static() is ok (with only one use of __FUNCTION__ in that line of code, it's recognizable what the parameter after it means).

Also, I'd encourage adding the 1-line comment to this patch (copy it from other uses). But I also wonder if we want to change that comment line to something else, maybe something that includes an @see drupal_static() directive. That's not for this issue, but the fact that this patch was rolled without including that 1-line comment makes me realize that the comment isn't sufficiently clear and/or useful.