CODEWITHSUNDEEP
phplaravel
user10074403
user10074403

Reputation:

Is it bad practise to use 2 models in one controller

We are using Laravel and we have multiple models controllers.

Ex:

Model1 and Model2 and they corresponding controllers Controller1 and Controller2. We need to make a new entry for Model2 every time we create new entry for Model1.

The question is, is it bad practice to make new entries in Model1 and Model2 in Controller1

public function store(Request $request)
{
    //Create new Model1
    $model1 = new Model1;
    $model1->bla = "Model1";
    $model1->save();

    //Create new Model2
    $model2 = new Model2;
    $model2->bla = "Model2";
    $model2->save();
}

Upvotes: 3

Views: 411

Answers (2)

kapitan
kapitan

Reputation: 2212

I have multiple projects that are doing what you just have said, saving entries on a multiple tables at once. I believe that this is really happening on an app in a real world scenario.

Here's what I am doing though:

public function store(Request $request)
{
    $main = $this->saveModel1($request);
    $this->saveModel2($request, $main->id);    

    return redirect()->route('my.route.name.for.edit', $main->id);
}

private function saveModel1($request)
{
    $model1 = new Model1;
    $model1->bla = "Model1";
    $model1->save();
    return $model1->id;
}

private function saveModel2($request, $model1_id)
{
    $model2 = new Model2;
    $model2->bla1 = $model1_id;
    $model2->bla2 = "Model2";
    $model2->save();
}

UPDATE: This answer has been selected as correct, but I just want to add that on condition like this, it is safe to enclose your saving query into a transaction.

DB::beginTransaction();
try
{

}
catch (\PDOException $e)
{

}

Upvotes: 3

Nabil Farhan
Nabil Farhan

Reputation: 1536

It is not good practice. Generally in OOP there is an important principle named 'Single responsibility Principle'. According to that, a function or a module should have only one responsibility. (Although originally it was a 'class' should have one responsibility.)

In my point of view better approach will be,

public function store(Request $request)
{
    Model1::StoreNewData();

    Model2::StoreNewData();
}

//And then in model1 and model2 implement the store function.

public static function StoreNewData()
{
    $model1 = new Model1;
    $model1->bla = "Model1";
    $model1->save();
    return;
}

Laravel specifically, you should strive for 'fat models, skinny controllers'. It will help you in the long run. For example, suddenly client tells you that you need to validate model1 and/or change the data or insert new data into ten more fields then it will be very hard to change. In real world there might be 20 fields for a table and in same url it will update 5 tables. Debugging it will be hard. But now you know where to change and you can easily go to specific function.

But originally you are using same function to store both model1 and model2. It should not be done. Although, countless tutorial works this way but in professional environment single responsibility principle is important.

I generally use controller functions only to redirect to another page. If there is validation and/or something then I'd use different function or trait.

You should look up SOLID principle in OOP and try to master it. Also check out this link.

Upvotes: -1

Related Questions