Hello.

Follow Drupal 7 architecture we should use drupal_static function
instead of static variables.

This allow external modules to clear static vars if needed.

Thanks.

CommentFileSizeAuthor
#2 nodequeue-use_drupal_static_for_variable_caching-1666556-2.patch2.67 KBCameron Tod
#1 nodequeue_qid_map_static.patch508 bytesigor.ro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

igor.ro’s picture

igor.ro CreditAttribution: igor.ro commented
FileSize
nodequeue_qid_map_static.patch508 bytes

Here is the patch.

Cameron Tod’s picture

Cameron Tod CreditAttribution: Cameron Tod commented
Status: Active » Needs review
FileSize
nodequeue-use_drupal_static_for_variable_caching-1666556-2.patch2.67 KB

Some code we are working on here would really benefit from this. static is used instead of drupal_static in several places, not just in nodequeue_get_qid_map().

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs review » Needs work

I need this to be able to run tests on nodequeues that are exported with Features Extra. For the moment I will have to implement my own variants of these functions so I can clear the static caches.

Review:

--- a/nodequeue.module
+++ b/nodequeue.module
@@ -521,7 +521,7 @@ function nodequeue_get_qids($type, $account = NULL, $bypass_cache = FALSE) {
     $account = $user;
   }
 
-  static $cache = array();
+  $cache = drupal_static(__FUNCTION__, array());
   if ($bypass_cache || !isset($cache[$type])) {
     $roles_join = $roles_where = '';
     $roles = array();

The ampersand is forgotten in this instance, causing the function not to be passed by reference.

Rest of the patch looks good.

  • fizk committed 8818a4e on 7.x-2.x 
    Issue #1666556 by igor.ro, cam8001: Use drupal_static instead of static...
fizk’s picture

fizk CreditAttribution: fizk commented
Issue summary: View changes
Status: Needs work » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.