Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was just writing code, and wanted to make a piece that removes everything after the character ? if it's found within the URL.

if (strstr($url, "?")) {
    $url = strstr($url, "?", true);
}

Can I make this any shorter? It doesn't feel right to type the "same thing" twice with strstr.

share|improve this question

We're looking for long answers that provide some explanation and context. Don't just give a one-line answer; explain why your answer is right, ideally with citations. Answers that don't include explanations may be removed.

2 Answers 2

You're using the same variable for two different purpose. It would be readable with separate variables:

$urlWithoutParameters = strstr($url, "?", true); 
if ($urlWithoutParameters !== FALSE) {
    // if you need additional processing
}

It might be safer to use PHP's built-in parse_url function which could help handling invalid inputs too:

On seriously malformed URLs, parse_url() may return FALSE.

From Effective Java, 2nd edition, Item 47: Know and use the libraries:

By using a standard library, you take advantage of the knowledge of the experts who wrote it and the experience of those who used it before you.

(It's mostly a Java book but it applies here.)

share|improve this answer

Doing a quick test, as follows, shows that using explode() is about 4 times as fast as calling strstr() twice.

$url = "http://google.com/test.php?somevar=laksd";
$url2 = "http://google.com/test.php?somevar=laksd";

$start = microtime();
for($i = 0; $i <= 2000000; $i++){
  if (strstr($url, "?")) {
    $url = strstr($url, "?", true);
  }
}
$end = microtime();

echo "took " . ($end - $start) . "<br>";

$start = microtime();
for($i = 0; $i <= 2000000; $i++){
  $url = explode("?", $url2);
  $url = $url[0];
}
$end = microtime();

echo "took " . ($end - $start) . "<br>";

print_r($url);

The output is as follows:

2000000 iterations of strstr took 0.573858
2000000 iterations of explode took 0.132764
http://google.com/test.php

However, unless you have hundreds of millions of visitors to your site, you don't have to hyper optimize this way. If its purely from an aesthetic point of view, then I would say just do:

$url = explode("?", $url);
$url = $url[0];

In PHP 5.4 and greater, you can access the array element directly (http://php.net/manual/en/language.types.array.php#example-88):

$url = explode("?", $url)[0];
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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