PHP function doesn't return value

Question

I have a function which, given a filename and directory path, checks if the directory already contains a file with the same name and if so returns an amended filename (by appending a number after the first part of the filename). (The get_filenames() function is a CodeIgniter helper function which creates an array of all the filenames in the specified directory.)

When I try to print out the returned result of the function call, I get nothing; but if I print $new_filename in the else{} statement of the function itself, then simply call the function (rather than printing it's value), it works!

I need to return the value in the function, not print it, as I actually need to assign the result to a variable for further processing. (In the example below I've just printed the result of the function call to demonstrate the point.)

The function:

function avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count)
{   
    $num = '';
    if ($count > 0):
     $num = $count;
    endif;

    $filename_arr = explode('.', $old_filename, -1);
    $new_filename = $filename_arr[0] . $num . '.' . $filename_arr[1];

    if (in_array($new_filename, get_filenames($dir))):  
     $count++;
     avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);
    else:
     return $new_filename;
    endif;
}

And where I call the function:

print avoid_conflicting_filenames('file.jpg', '', 'path/to/file', 0);

This has been driving me insane for the past day, so any help would be greatly appreciated! Thanks.

Answer 1

Replace this:

avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);

With this:

return avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);

You're not thinking about it recursively. You have to return the function's return value.

Past that, what's up with the if syntax? I tolerate it inside templates, but for code? ew.

If I'm understanding the code correctly, you could also rewrite this function to avoid recursion like so:

function avoid_conflicting_filenames($old_filename, $new_filename, $dir) {   
    $num = 0;
    $files = get_filenames($dir);
    $filename_arr = explode('.', $old_filename, -1);
    do {
        $new_filename = $filename_arr[0] . $num . '.' . $filename_arr[1];
        $num++;
    } while(in_array($new_filename, $files));
    return $new_filename;
}

I think this is nicer and a little bit easier to get, but it's up to you...

Answer 2

Change your code to:

function avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count)
{   
    $num = '';
    if ($count > 0):
        $num = $count;
    endif;

    $filename_arr = explode('.', $old_filename, -1);
    $new_filename = $filename_arr[0] . $num . '.' . $filename_arr[1];

    if (in_array($new_filename, get_filenames($dir))):          
        $count++;
        return avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);
    else:
        return $new_filename;
    endif;
}

You forgotten the return statment when calling avoid_conflicting_filenames inside of avoid_conflicting_filenames.

Answer 3

You're running the function recursively. You will need to pass the result on to the "parent" instance of the function:

if (in_array($new_filename, get_filenames($dir))):          
    $count++;
    // Note "return" statement below.
    return avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);
else:
    return $new_filename;
endif;

Answer 4

If the first branch in your second if is taken the function never encounters a return statement.

Change it to read:

if (in_array($new_filename, get_filenames($dir))):          
    $count++;
    //Added return
    return avoid_conflicting_filenames($old_filename, $new_filename, $dir, $count);
else:
    return $new_filename;
endif;

Answer 5

Doh! It's always the little things!! Thanks everyone.