1

This is a best practice question and not a specific issue. I'm fairly new to the MVC approach and Yii, and have developed on an app for a while now. I keep seeing talks on best practice and what to put in which file (controller, model, view, helper etc.) however i have not yet found anything specific in terms of examples.

I currently have calls like: Model::function() in my view files as well as checks like $var = app()->request->getParam(value, false);

I have calls in my controller file like Model::function() and Model::model()->scope1()->scope2()->findAll() I also think my controller files are getting a bit thick, but not sure how and where to put some of the bloat, i have been reading about the DRY and i think i'm not exactly DRY'ing my code so to speak. Could you give me a more clearer picture about what goes where, and suggestions or reasons why :) Appreciate any advice, thanks in advance.

here's an example call in a viewfile

<?php $this->pageTitle = 'Edit Action'; $this->subTitle = '<i>for</i> <b>' . Vendors::getName($_GET['vendor']) . '</b>'; ?> <div class="wrapper"> <?php echo $this->renderPartial('_form', array('model' => $model)); ?> </div>

The getName is my function in the model, is this a good way to call a function in a view?

Another example view file:

<div class="wrapper"> <?php if($this->action->id != 'create') { $this->pageTitle = "New Media Contact"; echo $this->renderPartial('_form', array('model'=>$model)); } else { $this->pageTitle = "New Vendor"; echo $this->renderPartial('_form', array('model'=>$model)); } ?> </div>

$model is set in the controller with type... Same question... could this be done.. cleaner..? better in terms of MVC and reusability/DRY?

EDIT After reading some of the responses here, esp. @Simone I refactored my code, and wanted to share what it looks like now...

public function actionCreate() { $model = new Vendors; // Get and Set request params $model->type = app()->request->getParam('type', Vendors::VENDOR_TYPE); $vendorsForm = app()->request->getPost('Vendors', false); // Uncomment the following line if AJAX validation is needed $this->performAjaxValidation($model); if ($vendorsForm) { $model->attributes = $vendorsForm; if ($model->save()) $this->redirect(array('/crm/vendors', array('type' => $model->type))); } $model->categories = Categories::getAllParents($model->type); $this->pageTitle = 'New ' . Lookup::item('VendorType', $model->type); $this->render('create', array( 'model' => $model, )); }

and the view create.php

<div class="wrapper"> <?php echo $this->renderPartial('_form', array('model'=>$model));?>

Thanks for all respnses

Improve this question
7
  • As your question stands, it will likely get closed. Maybe post a specific example instead? Commented May 31, 2012 at 23:00
  • you are already using Yii .. that by itself throws the best practices out the window Commented Jun 2, 2012 at 13:28
  • @tereško Please explain what you mean, yii doesnt use best practice? Which framework would you suggest to use instead? I have been looking at Symfony, and do like the amount of documentation etc. but yii has been very easy to work with for the most part... Commented Jun 2, 2012 at 18:55
  • @SeventySix , sure ... use of global state, use of eval(), use of tight coupling, use of @ for error suppression, deep inheritance hierarchies, not clear interfaces, computation in __construct() methods, use of static factory methods .. and so on Commented Jun 2, 2012 at 19:32
  • 1
    @teresko, this is above my current understanding... do you have a framework that has better practice you can recommend? Commented Jun 2, 2012 at 19:52

4 Answers 4

Reset to default
3

I am not too familiar with the Yii framework but can offer a few suggestions on a few specific things you mentioned:

Don't get too caught up with 'best practices' as like all design patterns MVC can be implemented and in certain cases interpreted in many different ways by different developers. So what does this mean? it means read up on MVC as much as you can, then simply just have a go :o) You will soon find out what slots where and why when you come up with a problem (which is normally along the lines of 'where does this belong, the controller or model?...'.

In terms of what goes where, you can google / search stackoverflow or read in countless books many explinations of what should do what and go where, but from the code snippet you provided I would suggest:

View files: (Unless this is a Yii specific thing) in my opinion your view files are a bit dirty. You are talking to the model directly (which is in fact the classical approach of MVC rather than some PHP app's adopting the 'controller is the only one allowed to speak to the model' method) but it appears your view is trying to get request data directly and for me this should not be anywhere near the view. The controller should be dealing with the request, using a model for validation and then passing the output into the view.

Model: This seems OK from the small snippet, but in general one important thing to remember is that the model != database (despite some people's suggestion that it is).

Controller: Again seems fine from your snippet, but to address your bloat in your controller, without seeing one of your controllers it would be hard to offer a suggestion. One thing that is always worth considering is the use of Services. Basically a service can be used to greatly simplify your controller by encapsulating a lot of repetitive / complicated model stuff. So instead of calling separate validation and persistence models within your controller, you just instantiate a service class, and it could just be a case of calling one method (which often returns a bool to indicate to your controller the success or failure of the operation) and then your controller just has to deal with what it does best (and should only do) the app's flow (i.e. redirect to another page, show error, etc).

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

7 Comments

Thank you STeve, ok so I learned about using $this for the controller, but if I am to call a different model from the controller as opposed to the one its linked to eg. Post postscontroller or in my case Vendor -> vendorscontroller but i need a category name label from model Category... how should i do that..? $this->getCategory() but then it seems I'm putting category specific code in the vendorscontroller...?? sorry if I'm not getting it...
You should try not to think of models being linked to controllers, it is true that a category model and controller would be found under the same folder / module in your app (for organisational purposes really). But models can be called in any controller, but you would need to read about the Yii way of doing that. Normally you would have to make sure your autoloader is set up for this. I think you should definitely do research on Services as these would simplify your controller.
Just thought my above comment on folder organisation may not be true for all apps. Some just use a general models, controllers folder, others use a more module way of organisation. There are many benefits to using MVC but always think of one of them as a way to just organise your code base in your app. I think you really need to have a firm grasp on OOP, this will make MVC much easier to understand.
Steve Thank you again for taking time to respond. I have developed my app inside a module in the modules folder of my app, i thought of doing it that way since the functions are all related, but am now starting to see how big it has become, and might not have understood the use of modules correctly... I will look into services as you suggested and see how i can maybe separate some functions for reusability.
Well modules are typically meant to be self-contained 'mini-apps' in that they should ideally be able to be interchanged between projects. Unfortunately not all frameworks enable this, however at a minimum modules should be used to organise your codebase into logical sections, such as 'blog', 'admin', 'frontend', but depending on the size of your app you may even want to break the modules down into smaller chunks (again the choice is yours). One of the benefits of using services is that it abstracts a lot of things away from your controller and helps to keep it lean.
|
2

I'll show you an'example to refactor your code. This is you code

<div class="wrapper"> <?php if($this->action->id != 'create') { $this->pageTitle = "New Media Contact"; echo $this->renderPartial('_form', array('model'=>$model)); } else { $this->pageTitle = "New Vendor"; echo $this->renderPartial('_form', array('model'=>$model)); } ?> </div>

First question is: why write two time the same line with renderPartial? First refactoring:

<div class="wrapper"> <?php if($this->action->id != 'create') { $this->pageTitle = "New Media Contact"; } else { $this->pageTitle = "New Vendor"; } echo $this->renderPartial('_form', array('model'=>$model)); ?> </div>

And now second step:

<?php $this->pageTitle = $this->action->id != 'create' ? "New Media Contact" "New Vendor"; ?> <div class="wrapper"> <?php echo $this->renderPartial('_form', array('model'=>$model)); ?> </div>

FOR ME is more readable. I thing there are a lot of best practice. But can become a bad practice used in bad context. So... Is really useful rewrite code? For me yes! Because my goal is maintainability of code. Easy to read, easy to manage. But you need to find your standard or your team standard. Also, I prefer move any kind of logic in controller. For example I can set a default pageTitle in controller and redefine It in actionCreate method:

class SomeController extends CController { public $pageTitle = "New Vendor"; function actionCreate () { $this->setPageTitle("New Media Contact") $this->render('view'); } }

And my viewfile will become just:

<div class="wrapper"> <?php echo $this->renderPartial('_form', array('model'=>$model)); ?> </div>

I think we have to understand the responsibility of things: view is just a view.

Improve this answer

2 Comments

Thank you Simone that was very helpful :) I'm picking things up as I go along, but it seems to me there is very little written on how to really do things well, all (simple) examples seem to be pretty bad practice, so how is one supposed to learn?
I looked back at my code after reading this, and tried to put all logic to determine the output in the controller, but also keep it as flexible if there were to be more types, so i used the Lookup::item('VendorType', $type) function with type coming in the request string. Now I'm facing problems with access control, since one role is allowed to access one type but not the other, but in yii it seems that access control is limited to actions, but not parameters...
0

Simply think of a view as a component that only display data. It shouldn't do database calls, interact with a model, create new variables (or very rarely), etc. If you want to do a check, or create an HTML block using some data, etc. use helpers for that purpose.
The data a view will display will come from a controller.

The controller is the maestro who'll do most of the work in your app: it will answer requests, ask the model for data if needed, pass the data to a view and render it, etc.

In your first example, simply save Vendors::getName($_GET['vendor']) in a variable, in your controller, and then pass it to the view.
Also, if you don't need all model's data, don't pass the whole object.

Regarding your second snippet, first of all you can move the echos out of the if statement because they are the same.
A good thing would be to do the check if ($this->action->id != 'create') in your controller, and give your view a simple boolean: 

if ($this->action->id != 'create') { // not sure if $this->action->id would remain the same, I don't know Yii $media = true; // or // $page = 'media'; } else { $media = false; // or // $page = 'vendor'; }

And the render the partial depending on the values returned by the controller.

Improve this answer

9 Comments

Thank you that is helpful, could you provide me an example of how you'd call the above function and how that is then passed to the view? Thanks again that makes more sense now :)
Sorry but I don't know how does Yii work, so I can't tell you about how to do stuff. I'm sure its documentation can help you though. Good luck! :)
I agree but I would say it's certainly not bad for the View to interact with a model on a read-only basis, definitely agree on the database calls directly from the view but simple model data - why bother the controller with this? Each to their own I guess ;o)
@SteveH If your views have some link with your models, you are loosing abstraction and not doing MVC anymore. Or at least, not doing popular MVC as PHP frameworks are supposed to do.
Well I think we'll have to agree to disagree on this (which I guess reinforces what I said to the OP - developer interpretation)
|
-3

In my opinion there is no "best practice". As long as you do not work in a team or want to release your script as open source, where dozens of people have to work with it, use the framework however you like and need it. And even if you work with others on the code there are no "god given" rules for that.

There are much more important things that matter than the question if you are "really allowed" to use a static function inside your view.

Improve this answer

5 Comments

Thank you, I am actually intending on building the app so that a team can get involved with the development, which is the reason i ask. Since I'd like to avoid too much re-writing when the time comes, I might as well start out on a good note...
If you are okay with doing things the wrong (the shitty?) way, at least don't encourage others to do the same.
@SamyDindane Wrong in your opinion? Wrong in my opinion? Wrong in the opinion of my friend Michael? In Opinion of the yii team or the wordpress team? Or whom? @ SeventySix As long as your team use the same guidelines, this will be no problem. If you use Yii, follow the Yii guidelines as good as possible.
@Oliver, yes thank you, but in your reference to "there is no best practice" I am speaking with a CIO from a huge firm with a million dollar dev budegt and he keeps telling me about best practice and separation of concerns etc. so there must be a way to learn this stuff and apply it...
@Oliver, I sense some hostility, I only ask because I'm interested in finding some advice, practical advice, and Stackoverflow has been a great help for me to find practical solutions to various roadbumps, as I consider it a great forum with talented individuals who take the time to help newbies like myself.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.