OOP php: Method recursion returns null

Question

I created a method for making auction names websafe, and i just added a check for dupplicate webnames, which basicly tries to fetch an auction with the given websafe name, then adds a random number to the webname. But the method ends up returning null...

private function dupplicateUrlFix($url){
    var_dump($url)//Correct
    $existingAuction = Auction::get($url, "webname");

    //if webname exists, cancatenate with random int, and check if new webname exists
    if(!empty($existingAuction->webname)){
        $newUrl = $existingAuction->webname.rand(0,9);
        $this->dupplicateUrlFix($newUrl);
        return;
    }
    var_dump($url) //Correct Not a dupplicate
    return $url;
}

public function get_url_clean($string) {
    $string = strToLower($string);
    //cleaning....
    //checks for dupplicate
    var_dump($this->dupplicateUrlFix($string)); //is null?
    die();

    return $clean;
}

I have tried switching the order in the dupplicateUrlFix() method and simply returning a string. It only goes into the if(!empty... one time.

Is there a solution for this, or a better approach?

I don't think $existingAuction->webname.rand(0,9) is valid. Perhaps do $newUrl = ($existingAuction->webname).rand(0,9) — Luke
– Luke, Commented Sep 18, 2014 at 17:20
@Doctus It makes more visual sense spaced as $newUrl = $existingAuction->webname . rand(0,9) <- concatenation. — Michael Berkowski
– Michael Berkowski, Commented Sep 18, 2014 at 17:21
@Doctus I didn't say your was invalid. I thought you implied it was invalid because the dot was intended as a class operator. I misread your comment. — Michael Berkowski
– Michael Berkowski, Commented Sep 18, 2014 at 17:23
Oh I see what you mean! Yeah I'm not exactly sure, but I know in the past when concatenating object properties with strings I've had to wrap them in brackets. — Luke
– Luke, Commented Sep 18, 2014 at 17:24

u_mulder · Accepted Answer · 2014-09-18 17:27:11Z

1

I think there's no need for recursion here:

private function dupplicateUrlFix($url) {
    $existingAuction = Auction::get($url, "webname");
    while (!empty($existingAuction->webname)) {
        $url = $existingAuction->webname . rand(0, 9);
        $existingAuction = Auction::get($url, "webname");
    }

    return $url;
}

answered Sep 18, 2014 at 17:27

u_mulder

54.8k5 gold badges54 silver badges72 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

DalekSall Over a year ago

That works! Thank you! it even looks alot cleaner :)

u_mulder Over a year ago

It can be even converted to do .. while so as not to write $existingAuction = ... twice)

Alex Howansky Over a year ago

You'll have an infinite loop when you hit the 12th name. And assuming the get() method hits a database, you'll have an infinite loop hitting the database.

u_mulder Over a year ago

Still we can add some counter to while to e.g. throw exception when number of attempts exceeded.

Alex Howansky Over a year ago

rand(0, 9) returns one character. So, you'll get the base name foo plus ten extra names foo0, foo1, etc. After eleven attempts, you have no more names to try.

Collectives™ on Stack Overflow

OOP php: Method recursion returns null

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related