We can easily remove most of the uses of array_shift() form menu.inc. As recent posts have noted, this is a very slow function because it requires re-indexing the whole array.

CommentFileSizeAuthor
#4 menu_no_array_shift-172764-4.patch1.17 KBchx
#3 no_shift_menu_2.patch1.34 KBpwolanin
no_shift_menu_1.patch1.22 KBpwolanin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work 
$a = array('foo', 'bar');
print next($a);

this prints bar so I think the use of next might not result in what we want. each could be, but then you need to list as well. I am fairly sure that this pair of functions is still (much) faster though.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

Ah, yes - stupid of me. I was confused by the PHP doc that says it works like current().

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
FileSize
no_shift_menu_2.patch1.34 KB

how about this instead?

chx’s picture

chx CreditAttribution: chx commented
Status: Needs work » Needs review
FileSize
menu_no_array_shift-172764-4.patch1.17 KB

And what do you think of this version?

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

looks pretty good - though we don't even need the $key in the foreach

owahab’s picture

owahab CreditAttribution: owahab commented
Status: Needs review » Reviewed & tested by the community

I think the existence of $key is good for readability.
Yet the code is RTBC IMHO.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Looks ok, committed.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)