CODEWITHSUNDEEP
phpurl
Ameer
Ameer

Reputation: 771

What is wrong with this loop code

So i wrote some code for making seo-friendly urls. These functions first make a seo-friendly slug and then if the slug already exists is the DB(in this case array) then they add a number with a dash next to it. if that also exists then they just +1 the number and then checks again and again...

eg. if i pass "title url" to the function. First it will convert it to "title-url" and if "title-url" already exists then it will add a number like "title-url-1" if it exists as well then it will +1 the number like "title-url-2" and then "title-url-3" and so on...

this is the code:

// CONVERTS STRING TO URL SLUG
function str_to_slug($str){
    $str = strtolower(trim($str));
    $str = preg_replace('/[^a-z0-9-]/', '-', $str);
    $str = preg_replace('/-+/', "-", $str);
    return $str;
}

// RETURN SLUG URL
function slug($title){
    $ori_url = str_to_slug($title);
    if( does_slug_exists($ori_url) ){ 
       return loop_slug_number($ori_url, 1); 
    }
    else{ 
       return $ori_url; 
    }
}

// ADD NUMBER
function loop_slug_number($slug, $number){
    if( does_slug_exists($slug.'-'.$number) ){ 
        loop_slug_number($slug, $number++); 
        exit; 
    }
    else{ 
        return $slug.'-'.$number; 
    }
}

// CHECKS WHEATHER THE SLUG EXISTS IN THE DB
function does_slug_exists($slug){
    $array = array("title", "title-0", "title-1", "title-2");
    return (in_array($slug, $array)) ? true : false;
}

i think everything should work fine. but when i echo slug("title"); i'm getting

Fatal error: Maximum function nesting level of '100' reached, aborting!

error line number is in the function does_slug_exists() on the 'return' line.

(the array is just for example i will use db validation.)

also if i replace the array with:

$array = array("title", "title-0", "title-2", "title-3");

then i get title-1 back.

Where is the mistake?

Upvotes: 1

Views: 191

Answers (3)

cmbuckley
cmbuckley

Reputation: 42468

Ignoring any comments about the code quality, the issue here is the post-increment of the $number variable. You can replace with:

return loop_slug_number($slug, ++$number);

However, I suggest that the entire function should be rewritten as a while loop as opposed to a pseudo-recursive function. In addition, it looks like a DB query is made upon each call of does_slug_exists(); I suggest you refactor this to make the query once and store the returned result set. Have a look at this example.

Upvotes: 1

Laur Ivan
Laur Ivan

Reputation: 4177

I'm not sure about PHP, but in C you should do a ++number instead. The idea is that the number gets incremented after the function was called if you do a number++ and before if you do ++number.

.. the joys of increment/decrement operators...

Upvotes: 0

David Schwartz
David Schwartz

Reputation: 182779

// ADD NUMBER
function loop_slug_number($slug, $number){
    if( does_slug_exists($slug.'-'.$number) ){ loop_slug_number($slug, $number++); exit;     }else{ return $slug.'-'.$number; }
}

This is really awful code. Instead of looping, use a while loop. Start the number at 0 and while the slug exists, increment the number.

Upvotes: 0

Related Questions