This is what I have:
function get_create
returns a link with new filtered/sorted properties:
function get_create ($new_query = array(), $script = null, $ext = true, $amp = true) {
$pr_url = $GLOBALS['pref_url'].$GLOBALS['host_name'];
$arr_query = array();
if (!empty($new_query)):
$new_query = array_filter($new_query, function($element) {
return !empty($element);}); //delete empty elements
if ($ext) {
$cur_url = str_replace("&", "&",$GLOBALS['cur_url'])
$str_query = parse_url($cur_url, PHP_URL_QUERY); // get string of queries
parse_str($str_query,$arr_query); // create an array with queries
$arr_query = array_replace_recursive($arr_query,$new_query)
$arr_query = get_sort_by_key($arr_query);
}
else {
$arr_query = $new_query; // if $new_query empty
}
//make a new link with new parameters
$fn_query = (!empty($arr_query) ? '?'.http_build_query($arr_query,'',($amp ? '&' : '')) : null);
$fn_script = (!empty($script) ? $script : $GLOBALS['script_name']);
return $pr_url.$fn_script.$fn_query; // return the link
else :
return $GLOBALS['home_url'];
endif;
} // end func
In parameters we have:
$new_query
(which contains the new properties for$_get
part)$script
(contain a new way to script, where data)$ext
(boolean given parameter which make the logic different)$amp
(if true then formats and again)
function get_sort_by_key
helps with sorting array (the keys in array must be in this order: lng-topic-page-sort-search).
function get_sort_by_key($array = array()) {
$new_array = array();
foreach ($array as $key => $value) {
$key = strtolower(trim($key));
switch ($key) {
case $key == 'lng': $order = 0;
break;
case $key == 'topic': $order = 1;
break;
case $key == 'page': $order = 2;
break;
case $key == 'sort': $order = 3;
break;
case $key == 'search': $order = 4;
break;
default: $order = 5; break;
}
$new_array[$order][$key] = $value;
}
ksort($new_array);
return $fn_array = call_user_func_array('array_merge',$new_array);
}
I need to create the function which gets an array full of new parameters and adds or replaces the current $_GET
by them in a specific order.
An example:
I had the URL like this: http://localhost/test.php
. After initialization func: get_create(array('sort' => 1, 'topic' => 2)
I get http://localhost/test.php?topic=2&sort=1
, where the order is important (not http://localhost/test.php?sort=1&topic=2
).
How can I make the function look better? My code works, at least now. But I know that I'm wrong. Maybe some elegant solution exists in my case.
-
2\$\begingroup\$ Is that your real code? There appear to be missing semi-colons on the 10th and 14th lines of the first function... \$\endgroup\$Sᴀᴍ Onᴇᴌᴀ– Sᴀᴍ Onᴇᴌᴀ ♦2017年12月18日 20:11:15 +00:00Commented Dec 18, 2017 at 20:11
-
\$\begingroup\$ If you fix the code so that it works, we'll re-open this question \$\endgroup\$Snowbody– Snowbody2017年12月19日 15:44:27 +00:00Commented Dec 19, 2017 at 15:44
-
\$\begingroup\$ Yes this is mine, the colons missing because I tried to make code more readable. \$\endgroup\$Medion– Medion2017年12月19日 17:45:07 +00:00Commented Dec 19, 2017 at 17:45
-
1\$\begingroup\$ @Medion Please don't edit your actual code to make it more readable, we are fully capable of reading code :) Is there anything else that you've edited / simplified before posting here? \$\endgroup\$Simon Forsberg– Simon Forsberg2017年12月19日 17:52:02 +00:00Commented Dec 19, 2017 at 17:52
-
1\$\begingroup\$ That's good to hear, however: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers . Feel free to post a follow-up question instead. \$\endgroup\$Mast– Mast ♦2017年12月19日 18:25:46 +00:00Commented Dec 19, 2017 at 18:25
2 Answers 2
How can I make the function look better?
One alternative approach to the switch
statement inside get_sort_by_key()
is to define a lookup-table using an array:
define('KEY_MAPPING', ['lng','topic','page','sort','search']);
(or it could be a constant defined on a class if such a relevant class exists).
Then the code block in the foreach
can be simplified using array_search()
to see if a corresponding index is returned. If not (i.e. it returned false
) then use 5 as the fallback value.
foreach ($array as $key => $value) {
$key = strtolower(trim($key));
$order = array_search($key, KEY_MAPPING);
if ($order === false) {
$order = 5;
}
$new_array[$order][$key] = $value;
}
See this playground example for a demonstration.
For small lists of parameters like this the time difference won't be much. I found this SO question from 6 years ago (and the results may have changed since Ariel's answer as PHP internals have changed but likely not by much).
I tried to find some online posts comparing switch
statements with array lookups. The first couple results I see in Google are 6-9 years old:
- TechnoSophos: Microbenchmarking PHP: Switch Statements are Slow
- DevShed: PHP: The Switch Statement and Arrays ("can they not indent??")
- ExceptionsHub: In PHP what's faster, big Switch statement, or Array key lookup
The general consensus is that the array lookup is faster. And typically it can require fewer lines of code. There is a quote somebody said (I can't remember who, so if you know who to give credit please let me know): "The easiest line to debug is the line you didn't write".
The default behavior of
array_filter()
is greedy about destroying falsey values.$new_query = array_filter($new_query, function($element) { return !empty($element);});
is identical in functionality to:
$new_query = array_filter($new_query);
I recommend consistently using curly braces on all constructs and logical tabbing to make your code more readable.
When a scalar or array type variable is sure to exist and you want to check if it is empty/non-empty or falsey/truthy, then don't use
empty()
. Just useif ($variable) {
to check. (e.g.$new_query
,$arr_query
, and$script
)For
get_sort_by_key()
, a key-based lookup will always outperform a value-based lookup (in_array()
orarray_search()
). Because php arrays are hash maps, key searches (array_key_exists()
orisset()
) are the way to go. Here is a basic demonstration of how easy and clean it is to execute a custom key sort: (Demo)$lookup = [ 'lng' => 5, 'topic' => 4, 'page' => 3, 'sort' => 2, 'search' => 1, // do not declare a key with a zero value ]; uksort( $input, function($a, $b) use($lookup) { return ($lookup[$b] ?? 0) <=> ($lookup[$a] ?? 0); } );
The above snippet uses descending values for your known keys. The zero value is deliberately excluded from the lookup because when the sorting algorithm finds a key which is not represented in the lookup, it is assigned a sorting value of
0
(it goes to the back of the array). The spaceship operator is a super-versatile tool. If you wanted to sort by your custom order THEN sort alphabetically in an ascending direction, you can simply declare two balanced arrays like this:return [$lookup[$b] ?? 0, $a] <=> [$lookup[$a] ?? 0, $b];
As for the maintainability of your code, whenever you want to adjust the priority of specific keys for custom sorting, you only need to touch your lookup array (not the uksort()
block). This makes readability and maintenance a breeze.