Returning a variable from a function to be used in another function

[piratemac541]

Hello. I’m using Sitepoint’s PHP Novice To Ninja book as a reference to build my own website framework.

I’m trying to add functionality to upload images, which I’ve done. However I was trying to move the PHP code into a reusable ‘framework’ function and have run into difficulties.

I have a form that someone adds a name and picture to and the code uploads the images and stores a refernce to it in the database.

I had it working on on my controller like this:

Blockquote

  public function add() {
    $author = $this->authentication->getUser();

    $item = $_POST['item'];
    //the above is from form, below is others
    $item['authorId'] = $author['id'];
    //upload files
        $file = $_FILES['file'];
        //print_r($file);
        //die;
        $fileName = $_FILES['file']['name'];
        $fileTmpName = $_FILES['file']['tmp_name'];
        $fileSize = $_FILES['file']['size'];
        $fileError = $_FILES['file']['error'];
        $fileType = $_FILES['file']['type'];

        $fileExt = explode('.', $fileName);
        $fileActualExt = strtolower(end($fileExt));

        $allowed = array('jpg', 'jpeg', 'png', 'pdf');

        if (in_array($fileActualExt, $allowed)){
            if($fileError === 0){
                if ($fileSize < 500000) {
                    $fileNameNew = $item['itemPicture'].'.'.$fileActualExt;
                    $fileDestination = 'uploads/'.$fileNameNew;
                    move_uploaded_file($fileTmpName,$fileDestination);
                    $item['itemFileName'] = $fileNameNew;
                } else {
                    echo 'Your file was too big! Reduce size to less than 500kb';
                }

            } else {
                echo 'There was an error uploading your file';
            }
        } else {
            echo 'This is not an allowed filetype! Convert to jpg or png';
        }
    //end upload files

    $this->itemsTable->save($item);

    header('location: /item/list');
}

This worked fine. So then I tried to move most of that code to my DatabaseTable of functions (where the save() function is) and call it like so:

Blockquote

public function add() {
    $author = $this->authentication->getUser();

    $item = $_POST['item'];
    //the above is from form, below is others
    $item['authorId'] = $author['id'];
    //upload files
    $this->itemsTable->upload($item['itemPicture']);
    //end upload files
    $item['itemFileName'] = $fileNameNew;

    $this->itemsTable->save($item);

    header('location: /item/list');

With the following function in the DatabaseTable file:

Blockquote

public function upload($value) {
	$file = $_FILES['file'];
        
        $fileName = $_FILES['file']['name'];
        $fileTmpName = $_FILES['file']['tmp_name'];
        $fileSize = $_FILES['file']['size'];
        $fileError = $_FILES['file']['error'];
        $fileType = $_FILES['file']['type'];

        $fileExt = explode('.', $fileName);
        $fileActualExt = strtolower(end($fileExt));

        $allowed = array('jpg', 'jpeg', 'png', 'pdf');

        if (in_array($fileActualExt, $allowed)){
            if($fileError === 0){
                if ($fileSize < 500000) {
                    $fileNameNew = $value.'.'.$fileActualExt;
                    $fileDestination = 'uploads/'.$fileNameNew;
                    move_uploaded_file($fileTmpName,$fileDestination);
					
                } else {
                    echo 'Your file was too big! Reduce size to less than 500kb';
                }

            } else {
                echo 'There was an error uploading your file';
            }
        } else {
            echo 'This is not an allowed filetype! Convert to jpg or png';
        }
		return $fileNameNew;

}

I get undefined variable errors for $fileNameNew after the upload function completes. The file uploads, the database is updated with the new entry with the exception of the itemFileName field

[tracknut]

$fileNameNew is a local variable within upload, and used as the return value of that function. As such, you need to assign it to something when you call upload(), or else it is lost. So your call from inside add() should look something like:

$item['itemFileName'] = $this->itemsTable->upload($item['itemPicture']);

You could also assign the return value to some other variable inside add rather than the $item array, but in this example it seems most expedient just to put it in the array where you want it.

[gillesmigliori]

Hi,

When you echo something it doesn’t end the function. So your function will try to return $fineNameNew in all cases, whereas $fineNameNew is set only in some specific conditions.

Rather than replacing your echo with return, it’s cleaner to set a variable and return it at the end of the function.

You should also wrap move_uploaded_file into a condition because it can fail.

Here’s how I’d write this:

public function upload($value) {
    $output = [
        'success' => false,
        'fileNameNew' => '',
        'message' => ''
    ];

    $file = $_FILES['file'];

    $fileName = $_FILES['file']['name'];
    $fileTmpName = $_FILES['file']['tmp_name'];
    $fileSize = $_FILES['file']['size'];
    $fileError = $_FILES['file']['error'];
    $fileType = $_FILES['file']['type'];

    $fileExt = explode('.', $fileName);
    $fileActualExt = strtolower(end($fileExt));

    $allowed = array('jpg', 'jpeg', 'png', 'pdf');

    if (in_array($fileActualExt, $allowed)) {
        if ($fileError === 0) {
            if ($fileSize < 500000) {
                $fileNameNew = $value.'.'.$fileActualExt;
                $fileDestination = 'uploads/'.$fileNameNew;
                if (move_uploaded_file($fileTmpName, $fileDestination)) {
                    $output['success'] = true;
                    $output['fileNameNew'] = $fileNameNew;
                } else {
                    $output['message'] = 'failed to move the file';
                }
            } else {
                $output['message'] = 'Your file was too big! Reduce size to less than 500kb';
            }
        } else {
            $output['message'] = 'There was an error uploading your file';
        }
    } else {
        $output['message'] = 'This is not an allowed filetype! Convert to jpg or png';
    }
    return $output;
}

Finally, I strongly advise you to adopt a coding standard, i.e., https://www.php-fig.org/psr/psr-12/

Many IDE extensions can lint your code according to these standards as you type.

[John_Betong]

Quite some time ago PHP introduced strict_types which I now use on every PHP file because type errors are thrown (for that file only) which need fixing before being able to continue. This makes debugging a lot easier:

The way declare(strict_types=1) works in PHP

From the article:

Using the strict_type declaration can help you catch bugs early on by restricting PHP to automatically cast primitive values. It may be a bit daunting when you start to use it, as you need to properly define everything from the start.

@piratemac541 I find your function is not intuitive and could be improved making the intentions obvious:

public function upload
(
string $value = ‘fileName without extension’
)
: string // fileName with correct extension OR error message
{
…
}

@gillesmigliori your function accepts a $value parameter and it is never used? Also could do with setting the return type to array. I’ve adopted the practice of setting and naming the $result immediately in the function knowing that the function will return $result;

Please also note that error_reporting(E_ALL); should be set in the php.ini file.

Finished editing on a tablet

[piratemac541]

Thanks for all of your help. Got it working nicely with some better handled error messages, and learnt a fair bit along the way : )