0

I need to loop through categories and then the top stores within each of the categories, and being relatively new to MVC I've unsure as to how to accomplish this in keeping with the MVC principles.

At the moment I was planning on doing something like this, but now I look at it I see an awful lot of stuff which I feel should be in the model:

My Controller looks like this:

public function category_list() {

  foreach ($this->CategoryModel->getCategoryList() as $cat) {           

    $data['cat_title'] = $cat['category_title'];        
    $data['list']['stores'] = $this->StoresModel->getStoresByCategory($cat['category_id']); 
    $this->_Load->view('stores_by_category.tpl', $data);

  }

}

Is this the right way around achieving this task or is my feeling of guilt correct?

Many thanks

Improve this question

5 Answers 5

Reset to default
2

Lot's of people still debate about questions like this one , 'fat model' v.s 'thin model' . if you ask me , you should consider you'r own code and decide where you are going to put you'r code , in such a way that in future you will have maximum use of you'r code and you'r application will be easily extensible. It's up to you , just make sure you wont push you'r code in the view. that's very wrong.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

The reason why a fat model is considered preferable is because it helps with re usability. If significant amounts of functionality are being implemented in the controller, then this tends to mean you can't move the model to another project or use it in another controller without copying chunks of the controller code over as well. Of course it's kind of a judgement call as to when this is worth it and when it's not, but as a rule of thumb, fat model thin controller is usually a good approach to follow
I agree , but in my practice when I have some algorithmic logic , or a bit complex logic , I'm separating that code in classes , ... I like to use the controllers like bridges between all the logic (models, helpers , other classes) ... but that's just the way I like to do things , it works fine for me.
1

The good:

You are using the controller to populate the data to be used in the view. You are making a model call to getCategoryList() vs. native sql, etc.

The bad:

Wrapping the template call within the loop. You should localize the the rows from getCategoryList() into $data like your doing, but then call the template once, with the $data array, then your template will iterate through $data.

Pointers:

Your controller should be as light as possible, however there is some wiggle room here. If you have business logic which dictates which categories to return, you need to determine if the logic will be accessed from multiple code points. If this is the case, I recommend you put the business logic into a centralized location (new class), which will be responsible for pulling in the model data, massaging it according to business rules, then returning the data to the calling code. If it's a one time business logic rule, then it is safe to put within the controller.

Improve this answer

2 Comments

So you're saying opt for a bridging class between the controller and model?
You got it! If there is any business logic which needs to be considered when presenting data via the view, and you want the logic to be accessible from multiple code-points. Some full-featured frameworks (Symfony) which use an ORM (doctrine/propel) actually have 'Table' or 'Peer' classes where you can put business logic.
1

You're right to want to avoid the Fat Controller, outside of Thomas the Tank engine you never want to see that guy. :)

In this particular case though, I'd be inclined to consider your code to be presentational, and therefore it is probably best placed in the view. Remember models should know nothing about how they're being used. For this reason I'd personally be inclined to assign $this->CategoryModel->getCategoryList() to a view variable and have the view implement the foreach loop.

Improve this answer

Comments

1

I don't understand why an instance of CategoryModel should take care of retrieving the categories' list. A model stores data about one entry in your database. What you're trying to do should be put in the model's repository class. The repository takes care of retrieving entries from the database. So, you should have two classes: CategoryModel and CategoryRepository.

The same goes for the StoresModel class. Additionally, a category's stores should be accessible through a CategoryModel instance like this: $category->getStores(). The stores property should be set by your ORM.

Also, you shouldn't inject models in your controllers (but I believe you're doing this because you're using models as if they were repositories).

Improve this answer

Comments

0

Your code looks fine, but if you really want your controller to ONLY bridge data from models to views, the foreach() loop should be done in the model, and your code should be something like:

/* controller side */
public function category_list()
{
    $this->_Load->view('stores_by_category.tpl', $this->StoresModel->getFormattedCategoryList());
}

/* model side */
public function getFormattedCategoryList()
{
    $data = Array();

    foreach ($this->getCategoryList() as $cat)
    {           
        $data['cat_title'] = $cat['category_title'];
        $data['list']['stores'] = $this->getStoresByCategory($cat['category_id']);
    }

    return $data;
}
Improve this answer

1 Comment

This seems like a sensible suggestion to me as it will stay local to the model whilst still maintaining the modularity/seperation of the model which MVC advocates.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.