Yev
Yev

Reputation: 2081

Placing logic inside controller vs view vs model

I'm generating a select menu with the all of the rows in a single model, and I have come up with 3 different solutions of doing this - but am having a hard time deciding on which one makes the most sense and follows MVC / Laravel best practices.

Model State

states table:
id    abbr    name
1     AL      Alabama
2     AK      Alaska
3     AZ      Arizona
4     AR      Arkansas
etc.

Solution #1: Retrieve all states from model in controller, perform logic to generate state array in controller passing it to view and generate select menu using laravels form class with passed in array.

Controller: my_controller.php

public function get_index()
{
    $states = State::all();
    foreach ($states as $state)
    {
        $states_array[$state->id] = $state->name;   
    }
        return View::make('my_view')->with('states_array',$states_array);
}

View: my_view.php

{{ Form::select('state_id',$states_array); }}

Solution #2: Retrieve all states from model in controller, pass retrieved state models to view, perform logic to generate state array in view and generate select menu using laravels form class with resulting array.

Controller: my_controller.php

public function get_index()
{
    $states = State::all();
    return View::make('my_view')->with('states',$states);
}

View: my_view.php

@foreach ($states as $state)
    $states_array[$state->id] = $state->name;
@endforeach
{{ Form::select('state_id',$states_array); }}

Solution #3: Add an all_array() (or optionally extend all) method to the state model which pulls all state records, performs logic to create an array and return the resulting array. Call said method in controller and pass it to the view. Generate select menu using laravels form class with passed in array.

Model: my_model.php

public static function all_array()
{
    $states = self::all();
    foreach ($states as $state)
    {
        $states_array[$state->id] = $state->name;   
    }
    return $states_array;
}

Controller: my_controller.php

public function get_index()
{
    $states = State::all_array();
    return View::make('my_view')->with('states',$states);
}

View: my_view.php

{{ Form::select('state_id',$states); }}

So which of these 3 solutions makes the most sense and follows MVC / Laravel best practices more closely? Optionally - is there a BETTER solution than the 3 outlined above?

Upvotes: 0

Views: 1301

Answers (3)

Jason Lewis
Jason Lewis

Reputation: 18665

I wouldn't use any of those solutions. You should implement the presenter pattern to wrap any presentation logic and keep it out of views. Keeping it a model is probably the next best way to go, but ideally you'd keep your domain logic separate from the presentation logic.

There are a few presenter bundles and Laravel 4 even has some presenter packages available.

Getting in the habit of separating your logic is great, especially when you realize you're doing a lot of presentation logic in your views.

But to be clear, the best way is to use the lists() method for this, as described above. Still give presenters a look though.

Upvotes: 0

Collin James
Collin James

Reputation: 9310

I would go with option one but simplify it using the lists() method from the model.

$states_array = State::lists( 'name', 'id' );

This produces the array you are looking for.

Upvotes: 2

Mike Brant
Mike Brant

Reputation: 71422

I would go with solution #3. There is no reason a model need only behave in an ORM-fashion. If retrieving a full list of states is a common activity you need to perform, then by all means add it to your model class. that way if you ever need to do anything like exclude certain states for instance, you can make that change in one single place.

Upvotes: 1

Related Questions