? Success
Pull Request for # 10942

User tests: Successful: Unsuccessful:

avatar rdeutz
rdeutz
27 Jun 2016

Pull Request for Issue #10942 .

Summary of Changes

replaced a not in 5.4 available function with some pure php code that should do the same thing

Testing Instructions

see #10670

avatar rdeutz rdeutz - open - 27 Jun 2016
avatar rdeutz rdeutz - change - 27 Jun 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jun 2016
Labels Added: ?
avatar rdeutz rdeutz - change - 27 Jun 2016
Title
Remove array column
Remove array_column function
avatar rdeutz rdeutz - change - 27 Jun 2016
Title
Remove array column
Remove array_column function
avatar brianteeman brianteeman - change - 27 Jun 2016
Rel_Number 0 10942
Relation Type Pull Request for
avatar bertmert
bertmert - comment - 27 Jun 2016

There's also an API function doing the same like array_column:
ArrayHelper::getColumn($redirects, 'old_url')

avatar rdeutz
rdeutz - comment - 27 Jun 2016

@bertmert because there is a native function I would depreciate the function, if we have a higher minimum PHP version it should go away. So I think a valid argument for using it is that we can identify the places where we have to replace code. For me not a great argument :-)

avatar ggppdk
ggppdk - comment - 27 Jun 2016

Why are we not using an array indexed by:
$redirect['old_url']
that points to object references of $redirect

and then use isset() ? it should be much faster

avatar mbabker
mbabker - comment - 27 Jun 2016

It looks like ArrayHelper::getColumn() can deal with nested arrays while array_column() is a single array only. I guess that's one reason to keep it but perhaps redo the internals to just use array_column().

/off-topic

avatar rdeutz
rdeutz - comment - 27 Jun 2016

@ggppdk I don't have a good feeling to use an url as an index for an array.

avatar brianteeman brianteeman - change - 27 Jun 2016
Category Plugins
avatar ggppdk
ggppdk - comment - 27 Jun 2016

@rdeutz

I don't have a good feeling to use an url as an index for an array.

i did not know that, i missed this,
do you have a link mentioning the dangers of it?

Understanding the PHP array implementation
the "index" , in our case url (string) will be hashed, and any collisions are handled with a linked list that holds the real "index" that was hashed

or do you mean something else ?

avatar rdeutz
rdeutz - comment - 27 Jun 2016

@ggppdk just a feeling without evidence, same level as I don't use spaces in a filename :-)

avatar ggppdk
ggppdk - comment - 27 Jun 2016

I see what you mean now,

any binary data can be used as index in PHP, hash function will just read bytes

In PHP, associative arrays are implemented as hashtables with some extra functions

[EDIT]
please anyone correct me here, if i am missing something

also this PR should work, (but i have not taken time to test it)

avatar brianteeman
brianteeman - comment - 27 Jun 2016

I assume that this can be closed as @eilsonge merged #10947 ??


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10943.

avatar wilsonge
wilsonge - comment - 27 Jun 2016

Yes - I was in the middle of typing that :) I've gone with the polyfill as it's effectively making our code optimised and forwards compatible

avatar wilsonge wilsonge - change - 27 Jun 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-06-27 21:35:24
Closed_By wilsonge
avatar wilsonge wilsonge - close - 27 Jun 2016

Add a Comment

Login with GitHub to post a comment