1
\$\begingroup\$

I'm trying to separate logic from the controller, so I created this Upload class. It is not a big class, just a class so that the controller doesn't know about the logic and stuff like that.

<?php 
namespace Dnianas\Uploads;

class Photo 
{    

    /**
     * The image filename.
     * @var string
     */
    protected $filename;

    /**
     * The image extension.
     * @var string
     */
    protected $extension;

    /**
     * The image hashed value that we hashed using SHA1.
     * @var string
     */
    protected $hashedName;

    /**
     * The filename with the hashed name and the extension.
     * @var string
     */
    public $path;

    public function makeProfilePicture($input)
    {
        $this->fileName     = $input->getClientOriginalName();
        $this->extension    = $input->getClientOriginalExtension();
        $this->hashedName   = sha1(time() . $this->fileName);
        $this->path         = $this->hashedName . '.' .$this->extension;

        // Resize the image
        $image = \Image::make($input);
        $image->fit(300, 300);

        // Set the destination
        $destination = photos_path() . '/' . $this->path;

        // Save the image
        $image->save($destination);

        // Return the object
        return $this;
    }

    public function makeCoverPhoto($input)
    {
        $this->fileName     = $input->getClientOriginalName();
        $this->extension    = $input->getClientOriginalExtension();
        $this->hashedName   = sha1(time() . $this->fileName);
        $this->path         = $this->hashedName . '.' .$this->extension;

        // Resize the image
        $image = \Image::make($input);
        $image->fit(900, 350);

        // Set the destination
        $destination = photos_path() . '/cover-' . $this->path;

        // Save the image
        $image->save($destination);

        return $this;
    }

}

As you can see, the first 4 lines from each method is exactly the same. How can I improve this?

EDIT: I've followed the constructor method. I added the constructor.

Here is the final class:

<?php 
namespace Dnianas\Uploads;

use Symfony\Component\HttpFoundation\File\UploadedFile;

class Photo 
{    

    /**
     * The image filename.
     * @var string
     */
    protected $filename;

    /**
     * The image extension.
     * @var string
     */
    protected $extension;

    /**
     * The image hashed value that we hashed using SHA1.
     * @var string
     */
    protected $hashedName;

    /**
     * The filename with the hashed name and the extension.
     * @var string
     */
    public $path;

    /**
     * The input from the user, Usually it's the file.
     * @var object
     */
    public $input;

    public function __construct(UploadedFile $input)
    {
        $this->input        = $input;
        $this->fileName     = $this->input->getClientOriginalName();
        $this->extension    = $this->input->getClientOriginalExtension();
        $this->hashedName   = sha1(time() . $this->fileName);
        $this->path         = $this->hashedName . '.' .$this->extension;
    }

    public function makeProfilePicture()
    {
        // Resize the image
        $image = \Image::make($this->input);
        $image->fit(300, 300);

        // Set the destination
        $destination = photos_path() . '/' . $this->path;

        // Save the image
        $image->save($destination);

        // Return the object
        return $this;
    }

    public function makeCoverPhoto()
    {
        // Resize the image
        $image = \Image::make($this->input);
        $image->fit(900, 350);

        // Set the destination
        $destination = photos_path() . '/cover-' . $this->path;

        // Save the image
        $image->save($destination);

        return $this;
    }

}

And in the controllers I do this:

$profilePicture = (new Photo($input))->makeProfilePicture();
\$\endgroup\$
  • 1
    \$\begingroup\$ Try using a constructor? \$\endgroup\$ – haakym Apr 14 '15 at 13:09
  • \$\begingroup\$ I guess this is how we all learn ;). You're welcome. See answer below for a couple of suggestions. \$\endgroup\$ – haakym Apr 15 '15 at 8:22
2
\$\begingroup\$

Add a constructor. I'm pretty sure you can type hint your dependency too, so long as you know what you're working with, you can adjust to your needs.

use Symfony\Component\HttpFoundation\File\UploadedFile;

class Photo {    

    protected $fileName;
    protected $extension;
    protected $hashedName;
    protected $path;

    public function __construct(UploadedFile $input)
    {
        $this->fileName     = $input->getClientOriginalName();
        $this->extension    = $input->getClientOriginalExtension();
        $this->hashedName   = sha1(time() . $this->fileName);
        $this->path         = $this->hashedName . '.' .$this->extension;
    }

    // methods...

}

Please also note in your code you wrote protected $filename; then referenced back to that in your methods as $this->fileName - make sure you get the n or N consistent.

You can always make a getter for the path attribute instead of making it public keep it protected.

public function getPath()
{
    return $this->path;
}

Hope that helps!

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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