Thursday, 30 August 2018

The PHP function does not return a value

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.

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...

0 comments:

Post a Comment