2
\$\begingroup\$

This is what I have:

function get_create returns a link with new filtered/sorted properties:

Code on screenshot

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:

  1. $new_query (which contains the new properties for $_get part)
  2. $script (contain a new way to script, where data)
  3. $ext (boolean given parameter which make the logic different)
  4. $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).

Code 2 on screenshot

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.

\$\endgroup\$
  • 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ᴇᴌᴀ Dec 18 '17 at 20:11
  • \$\begingroup\$ If you fix the code so that it works, we'll re-open this question \$\endgroup\$ – Snowbody Dec 19 '17 at 15:44
  • \$\begingroup\$ Yes this is mine, the colons missing because I tried to make code more readable. \$\endgroup\$ – Medion Dec 19 '17 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 Dec 19 '17 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 Dec 19 '17 at 18:25
1
\$\begingroup\$

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',array('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:

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".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.