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 have a URL that is hardcoded via a config which I extract into a config:

$url = 'http://www.example.com/api/v1/all/limit/{limit}/offset/{offset}' 
// this is stored as a string inside a config file

then $limit = 10; $offset = 0;

I currently use the following preg_replace to replace the given URL and transform into an effective URL:

$url = preg_replace('/\{([A-Z, a-z]+)\}/e', "$$1", $url);

The effective string is now:

http://www.example.com/api/v1/all/limit/10/offset/0

It works, however, I had to dig deep into figuring out the pattern and $$1 part so there goes readability.

My questions:

  1. Is there anything else out there that someone can think of that can be as good as the above code?

  2. I could use a str_replace with 2 arrays and do a replacement of the variables - it might be cleaner but it maybe slower. Would anyone advise replacing that theory instead of this? If so, why?

  3. The number of variables could grow - currently doing this it will capture all $variable there is within that scope - so any suggestion would need to keep in mind that limit/offset will not be the only limited variables that will be incoming:

    $url = 'http://www.example.com/api/v1/all/id/{id}/limit/{limit}/offset/{offset}'
    

    $url = 'http://www.example.com/api/v1/all/relatedId/{relatedId}/limit/{limit}/offset/{offset}'
    
share|improve this question
1  
I suspect that by [A-Z, a-z], you really mean [A-Za-z]. The former will match characters as well as commas and spaces. In general, recall that a-d is short for abcd in character classes, so [A-Da-d] becomes [ABCDabcd], which is right, while [A-D, a-d] becomes [ABCD, abcd], which is probably wrong. –  Schism Jul 17 at 14:28
add comment

3 Answers 3

up vote 2 down vote accepted

If you're going after readability and speed, then doing a preg_replace is actually a lot slower than doing a str_replace.

I'm using this test code:

$original_url = 'http://www.example.com/api/v1/all/limit/{limit}/offset/{offset}' ;
$limit = 10;
$offset = 0;

$preg_start = microtime();
$url = preg_replace('/\{([A-Z, a-z]+)\}/e', "$$1", $original_url);
$preg_end = microtime();

$preg = $preg_end - $preg_start;

$str_start = microtime(true);
$url = str_replace(array("{limit}", "{offset}"), array($limit, $offset), $original_url);
$str_end = microtime(true);

$str_a = $str_end - $str_start;

$str_start = microtime(true);
$url = str_replace("{limit}", $limit, $original_url);
$url = str_replace("{offset}", $offset, $url);
$str_end = microtime(true);

$str_b = $str_end - $str_start;

echo $preg . "\n" . $str_a . "\n" . $str_b . "\n";
echo '$preg > $str_a: '  . ($preg > $str_a ? "true" : "false")  . "\n";
echo '$preg > $str_b: '  . ($preg > $str_b ? "true" : "false")  . "\n";
echo '$str_a > $str_b: ' . ($str_a > $str_b ? "true" : "false") . "\n";

And I'm getting these results:

6.4000000000008E-5
1.4066696166992E-5
3.0994415283203E-6
$preg > $str_a: true
$preg > $str_b: true
$str_a > $str_b: true

So, ultimately, using two function calls to str_replace() without arrays is faster overall than preg_replace and str_replace with an array.

EDIT

Considering your newly added third point (non-fixed number of variables), it would be best to abstract this to a function, at the sacrifice of speed.

function replace(array $replacements, $url){
    foreach($replacements as $template => $value){
        $url = str_replace($template, $value, $url);
    }

    return $url;
}

$url = 'http://www.example.com/api/v1/all/relatedId/{relatedId}/limit/{limit}/offset/{offset}';
$replacements = array("{relatedId}" => "231", "{limit}" => "10", "{offset}" => "10");

$start = microtime(true);
echo replace($replacements, $url);
$end = microtime(true);

echo "\n" . ($end - $start);

Results in:

http://www.example.com/api/v1/all/relatedId/231/limit/10/offset/10
2.4795532226562E-5

Which is comparable in speed to the multiple str_replace call (which is what you're technically doing in the function.

share|improve this answer
    
tks for giving your speed input. Note though I have updated a 3rd bullet to the optimization requirement where limit/offset is just an example, it may grow to 3-4 variables. As far as I know str_replace will be as slow as preg_replace if variables grows. But +1 for pointing out the speed. –  azngunit81 Jul 16 at 19:37
    
Added an edit to address item #3 –  jsanc623 Jul 16 at 20:22
add comment

You could simplify to something such as:

$url = 'http://www.example.com/api/v1/all/limit/%d/offset/%d';

$newURL = sprintf($url, $limit, $offset);

We use sprintf() for this.

Update

e (PREG_REPLACE_EVAL)

Warning This feature has been DEPRECATED as of PHP 5.5.0. Relying on this feature is highly discouraged.

Docs

Now I really suggest you find an alternative.

According to your update, you say the number of variables might grow. Could you explain (example code?) this more? If it "grows", then the issue may not be this part of the function, it may be the surrounding code that needs fixing.

share|improve this answer
    
Ill take a look at it - however your reference to php.net manual has a poor way to explain it. After a bit more searching on sprintf(), this stackoverflow link serves as a better enlightenment in why sprintf is a better situation function in this case: stackoverflow.com/questions/1386593/… –  azngunit81 Jul 16 at 17:28
1  
I'm glad that I could inspire some creative research, sorry the docs didn't do it for ya! It's always best to vote (up or down) on any answers you receive. If you feel my answer helped the most, accepting it is encouraged in our community :) –  Alex L Jul 16 at 17:34
    
I'll take it into consideration while I wait for other possible answers and possibility. I mean its a working code, just picking for best optimization. –  azngunit81 Jul 16 at 17:45
    
Of course, I must say though, I can't think of many other options, and I'd be shocked if there was something more efficient than a built-in function. –  Alex L Jul 16 at 19:22
1  
PREG_REPLACE_EVAL~ is a PCRE modifier for preg_replace(). It's the e` after the regular expression that causes the evaluation. –  Alex L Jul 16 at 21:49
show 2 more comments

Hardcoded url templates are not a way to go imo, because they force you to assign expected variables - every url change happens in at least 2 places. You should use some build function that won't care about template, but build url that app currently needs. Simplified example:

function addUrlParameters($url, $parameters) {
    foreach ($parameters as $key => $value) {
        $url .= '/' . $key . '/' . $value;
    }
    return $url;
}

$base = 'http://www.example.com/api/v1/all';
$parameters = array(
    'limit'  => 10,
    'offset' => 30
);

echo addUrlParameters($base, $parameters);

foreach loop might do much more then just add /key/value pairs. If in future your value needs to be urlencoded string you could implement it within function.

share|improve this answer
    
Hardcoded URL is important for API calls which is the whole purpose of this. YOU WANT expected values and parameters only and return 404 when expected that you made a mistake in the URL. Im sorry your logic doesn't fit the optimizations needs. –  azngunit81 Jul 17 at 12:21
    
You can't hardcode API calls, because they don't depend on application but the user. The code you gave have nothing to do with url/request validation either (maybe this is why nobody has got your intentions right). Right now it looks like you're protecting yourself against your own app giving invalid request string for the user. User might change it anyway and your application is doing what your code tells it to do - it won't get self aware anytime soon. –  shudder Jul 17 at 13:40
    
Again, URL for API calls are FIXED with varying structure. I'm calling the shots as far as what variables are necessary in order to activate the URL. So far, the answers stated have valid points which helps the question at hand. –  azngunit81 Jul 17 at 13:48
add comment

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.