Passing and returning string

[NotSureILikePHP]

Okay this should be extremely simple and maybe I'm an idiot or just forgetting something simple. It has been a while since I programmed and never in PHP. I am working on an open source project where the coders are horrible and they create a paging object to go to page 2 or page 3 or whatever page you click on. However they never remove old variables in the url so for example if I click on page 2 the url will show

 

http://www.whatever.com/page.php?p=2&order_by=name

 

Then when I click on page 3 it will show

 

http://www.whatever.com/page.php?p=2&order_by=name&p=3&order_by=name

 

and so on and there are alot more variables so the url gets unreadable really quickly.

 

I want to build a function that removes this to make the code more readable so I did this

 

 

 
$file = removeCrap($this->url);
 
function removeCrap($file) {
echo 'Here';
 
//these are some of the fields I am removing.
if (strpos($file,'&_pjax=%23pjax-container')) {
            $file = str_replace('&_pjax=%23pjax-container','',$file);
        }
        if (strpos($file,'&gridOrder=cust_name%20DESC%20')) {
            $file = str_replace('&gridOrder=cust_name%20DESC%20','',$file);
        }
        if (strpos($file,'&gridOrder=cust_name%20ASC%20')) {
            $file = str_replace('&gridOrder=cust_name%20ASC%20','',$file);
        }
        if (strpos($file,'p=')) {
            $file = str_replace('p=','',$file);
        }
        if (strpos($file,'&order=ASC')) {
            $file = str_replace('&order=ASC','',$file);
        }
return $file;
}
 

 

I have commented out the return to test to see if that helps but my code never reaches the echo statement. If I remove the function and just place it in the same function that's building the url for the pages it works but clutters up the function. Why can't I create a separate function to make the code cleaner?

 

Also, any suggestions on removing page numbers would help too. I can just loop through page 1 to 100 and remove them if they exist but I would rather just be able to know exactly what to remove. For example:

 

 

http://www.whatever.com/page.php?p=2&order_by=name

 

Since I don't know what page number to remove in the code because it could be page 1 or 50 I would want to know to remove p=2 without having to loop through 100 numbers just to clean up a url.

[mac_gyver]

this shouldn't be a matter of removing incorrect information after the fact, but of building the correct information in the first place, especially since the url that's being built is invalid with multiple sets of same name keys and could easily end up being longer than what browsers/servers accept for url lengths.

 

it sounds like the pagination code is trying to append things to the url, rather than to just build the url from any existing get parameters and the page number it is responsible for.

 

if you do an advanced search on the forum (the snow-flake thing to the right of the search box) and search for http_build_query and my user name as the author, you will find a page of results that show how to use http_build_query() to properly take any existing get parameters, without needing to know what they are or if there are any at all, and allowing the pagination code to just set the page value that is is responsible for and produce the pagination links.

[QuickOldCar]

Along with http_build_query() as mac_gyver said...you can access the $_GET array and unset any do not want, or even do a whitelist array and use in_array() to exclude to only allow those. Always grab the page value and do w/e pagination do on it, like a +1 or -1.

 

Simple example

<?php
$whitelist = array(
    "about",
    "category",
    "id",
    "page"
);

if (isset($_GET['page']) && ctype_digit($_GET['page'])) {
    $page = $_GET['page'];
    unset($_GET['page']);
} else {
    $page = 1;
}

if ($page <= 1) {
    $page = 1;
}

foreach ($_GET as $key => $value) {
    if (!in_array(strtolower($key), $whitelist)) {
        unset($_GET[$key]);
    }
}

if (empty($_GET)) {
    $amp = "";
} else {
    $amp = "&";
}

$previous     = $page - 1;
$next         = $page + 1;
$query_string = "?" . http_build_query($_GET, '', '&') . $amp . "page=";

if ($previous >= 1) {
    echo "<a href='" . $query_string . $previous . "'>Previous</a> ";
}
echo "<a href='" . $query_string . $page . "'><b>$page</b></a> ";
echo "<a href='" . $query_string . $next . "'>Next</a> ";
?>

[requinix]

Along with http_build_query() as mac_gyver said...you can access the $_GET array and unset any do not want, or even do a whitelist array and use in_array() to exclude to only allow those. Always grab the page value and do w/e pagination do on it, like a +1 or -1.

I feel iffy about making changes to $_GET and $_POST. How about making a copy and modifying that instead?

[QuickOldCar]

Sure I can see a few reasons why not to. Can't get them back for one.

 

Also doing a specific check for post or get may not always work here if a post form was submitted for a url with a get variable in it.

 

I suppose using $_REQUEST isn't bad to do since holds all the values and have the whitelist.

 

If was any concerns can make more checks and rules for it.

<?php
$request = $_REQUEST;

$whitelist = array(
    "about",
    "category",
    "id",
    "page"
);

if (isset($request['page']) && ctype_digit($request['page'])) {
    $page = $request['page'];
    unset($request['page']);
} else {
    $page = 1;
}

if ($page <= 1) {
    $page = 1;
}

foreach ($request as $key => $value) {
    if (!in_array(strtolower($key), $whitelist)) {
        unset($request[$key]);
    }
}

if (empty($request)) {
    $amp = "";
} else {
    $amp = "&";
}

$previous     = $page - 1;
$next         = $page + 1;
$query_string = "?" . http_build_query($request, '', '&') . $amp . "page=";

if ($previous >= 1) {
    echo "<a href='" . $query_string . $previous . "'>Previous</a> ";
}
echo "<a href='" . $query_string . $page . "'><b>$page</b></a> ";
echo "<a href='" . $query_string . $next . "'>Next</a> ";
?>