freeza's avatar

Solid Princip Problems

Hello guys, i created a ImageUploader Class where Image Uploads (store, thumbnail) will be handeled.

Also i try to use the SOLID princip and i think i have some problems here...

My questions:

  • Is the "ImageUploader"- Class really the right place for the function "thumbnail" (generation)? -->I think its not the task of the "ImageUploader"- Class or is it ok? How could i do it better?

  • Is it okay that i am using "UploadedFile"- Class as dependency? Also the Image Class? I need them for Upload but is that dependency ok?

  • Is it okay that i use laravels config helper function to configure the image paths in my constructor?

Sorry i have some problems with the solid princip.

<?php
namespace App\Helpers;

use Intervention\Image\Facades\Image;
use Symfony\Component\HttpFoundation\File;

class ImageUploader {

    private $originalName;
    private $thumbnailName;
    private $path;

    public function construct(UploadedFile $file)
    {
        $this->file = $file;    
        $this->path = config('file.img.storage');
        $this->setThumbnailName();
        $this->setOriginalName(); 
    }

    public static function fromForm(UploadedFile $file)
    {
        return new static($file);
    }

    private function setThumbnailName()
    {
        $this->thumbnailName = 'thumb_' . time() . '_' . $this->file->getClientOriginalName();
    }

    private function setOriginalName()
    {
        $this->originalName = time() . '_' . $this->originalPrefix . $this->file->getClientOriginalName();
    }

    public function thumbnail($width = 400, $height = 400)
    {
        $thumb = Image::make($this->path . $this->originalName);
        $thumb->resize($width, $height);
        $thumb->save($this->path . $this->thumbnailName);
    }

    public function store()
    {
        $this->file->move($this->path, $this->originalName);
        return $this;
    }       

}
?>
0 likes
2 replies
ohffs's avatar
ohffs
Best Answer
Level 50

It looks fine to me :-) Would it make you happier if it was called something like 'ImageProcessor' rather than 'ImageUploader'? One thing is you might want to sanitise the data from getClientOriginalName() - even something like preg_replace('/[^a-zA-Z0-9]+/' ... would help :-)

freeza's avatar

ah ok thank you!! ;) Yeah ImageProcessor sounds better.

Please or to participate in this conversation.