Validate avatar image

Question

I want to know if that code is correct and readable.

In class avatar the method getAvatar() validates the data. It checks if the field is not empty and if the photo extension is correct. If not, an appropriate exception is thrown. Class AvatarUpdating is responsible for updating the avatar in the database.

Avatar validation

declare(strict_types=1);

class Avatar
{
    public function __construct(
        private $data,
    ){}

    public function getAvatar(): string
    {
        $val = $this->data['avatar'];

        if(empty($val))
        {
            throw new ValidationException("Choose picture");
        }
        else if(!in_array($this->getExtension(), ['jpg', 'png', 'jpeg']))
        {
            throw new ValidationException("Incorrect extension");
        }
        return $val;

    }

    private function getExtension(): string
    {
        $val = $this->data['avatar'];

        return pathinfo($val, PATHINFO_EXTENSION);
    }
}

class ValidationException extends Exception {}

Avatar update

declare(strict_types=1);

class AvatarUpdating
{

    public function  __construct(
        private $database,
    ){}

    public function UpdateAvatar(int $id, string $photo): void
    {
            $query = $this->database->ConnectDatabase()->prepare("UPDATE user set avatar = :avatar where user_id = :id");
            $query->bindParam(':avatar', $photo, PDO::PARAM_STR);
            $query->bindParam(':id', $id, PDO::PARAM_INT);
            $query->execute();
    }
}

view

$avatarValidation = new Avatar($_POST);
$update_avatar = new AvatarUpdating($database);

div class="row">
            <div class="col-8 col-sm-8 col-md-8 col-lg-8 col-xl-8">
                <div class="custom-file">
                    <input type="file" class="custom-file-input" id="customFileLang" lang="pl-Pl" name="avatar">
                    <label class="custom-file-label border border-dark" for="customFileLang">Wybierz awatar</label>
                    <span class="error">
                        <?php
                        if(isset($_POST['submit_avatar']))
                        {
                            try {
                                    $update_avatar->UpdateAvatar($_SESSION['user_id'], $avatarValidation->getAvatar());
                                }catch (ValidationException $exception)
                                {
                                    echo $errorMessage = $exception->getMessage();
                                }
                        }
                        ?>
            </span>
                </div>
            </div>
            <div class="col-4 col-sm-4 col-md-4 col-lg-4 col-xl-4">
                <button type="submit" class="btn btn-danger mb-3" name="submit_avatar">Dodaj</button>
            </div>
        </div>

Is the code readable to get the first job?

Answer 1

Make unique exception sub-classes

It is good that ValidationException is declared as a subclass of Exception so that individual type of exception can be caught instead of all exceptions, though it would be better to make separate unique sub-classes - e.g. EmptyAvatarException and InvalidExtensionException. That way the error message can exist in the front-end coded and the back-end code won't have to deliver the message. One advantage of this is that the front-end could display messages in different languages if necessary.

Follow PHP Standards Recommendations for readability

While there is no de-facto rules about readability conventions and it is up to individuals/teams to decide their conventions, idiomatic PHP code often follows the PHP Standards Recommendations - e.g. PSR-12: Extended Coding Style. The code presented follows many of the recommendations though not all. For the complete list see this analysis result. The first thing I had noticed before doing the analysis was spacing after keywords like if:

if(empty($val))

Idiomatic PHP code has a space after the if:

if (empty($val))

And opening braces for conditional blocks don't need to go on a separate line - that is only for functions, methods and class definitions:

if (empty($val)) {