1
\$\begingroup\$

I just care if it's any of the common formats so that later when I list all files, for those that are images, I can generate previews based on mime type.

So PHP.NET suggest to use exif_imagetype function to check if file is an image. In case of success an integer will be returned indicating one of 18 constants.

The check I do is against an UploadedFile so my method is quite simple. As this is code review. Would you optimize my code to be even better? Thanks for any hints.

/**
 * @link https://www.php.net/manual/en/function.exif-imagetype.php
 *
 * @param UploadedFile $file
 * @return bool
 */
private function isFileImage(UploadedFile $file): bool
{
    $result = exif_imagetype($file->getRealPath());

    return is_int($result) && $result >= 1 && $result <= 18;
}

enter image description here

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

The method in question does not use $this therefore you might consider making it static.

Further it is a private method, i dont know of which class, but consider making it a public method of its own class.

The method accepts UploadedFile but it is only interested in the real path. As long as you keep it private it could be ok and simpler to call, but if you ever make it public (as suggested above), you should let it accept string path instead, that way it will be more flexible.

There are 18 constants for the image types, but you are using hardcoded numbers. What If another type is added in the future? Do you want to have to modify your code? Check the exif_ imagetype return value doc. It will never return int that is not one of the constants. return is_int($result); is absolutely enough.

\$\endgroup\$
1
\$\begingroup\$

I agree with the advice in slepic's answer.

If you want to merely ensure that the file is an image, then you could ensure that the return value from the call to exif_imagetype() is not FALSE:

return exif_imagetype($file->getRealPath()) !== FALSE;

That way if more constants are ever added to the list of IMAGETYPE_* constants you would not need to update the conditionals in this method.


Curiosity got the better of me and I did a quick search online, finding this SO post. The accepted answer suggests using mime_content_type() or finfo_open() depending on the PHP version used, though with those approaches you would likely need to utilize a string function to check that the mime type contains 'image', which seems to be slower than just calling the exif_imagetype() function in my local testing.

It also mentions that exif_imagetype is an alternative but relies on having the appropriate libraries enabled (which you presumably do) and only returns image mimetypes.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.