Laravel - php constructor

Question

I have a controller called "EncodeController". In this controller, I wrote a function that get's the original file path stored in the database, from a table called "uploads_table". Then, it converts the file with ffmpeg, and when the conversion is done, I want the controller to store in a new table "encodes_table" information about the encoded version of the file.

In order to get access to the tables I use the __construct functions, like this:

protected $upload;
protected $encode;

public function __construct(Upload $upload) {

    $this->upload = $upload;
}

public function ___construct(Encode $encode) {

    $this->encode = $encode;
}

I have managed to get that working, but the question is; Will that be considered as bad practice?

Update: I changed my constructor to:

protected $upload;
protected $encode;

public function __construct(Upload $upload, Encode $encode) {
    $this->upload = $upload;
    $this->encode = $encode;
}

Answer 1

Looking at the code the way it is, there's nothing wrong, but looks like your code will grow and something like this might be considered a bad practice:

protected $upload;
protected $encode;

public function __construct(Upload $upload, Encode $encode) 
{
    $this->upload = $upload;

    $this->encode = $encode;
}

public function upload()
{
   $file = ffmpegencode($this->upload->getFile());

   $this->encode->file = $file;

   $this->encode->uploaded_at = new \Carbon\Carbon;

   $this->encode->save();
}

Controller knowing too much about your models it's not a desired coupling. Instead you should so something like:

public function upload()
{
   if( ! $this->encode->encodeUploadedFile($this->upload->getFile()))
   {
      return Redirect::to('error');
   }

   return Redirect::to('success');
}