mvc best practice view/controller/model in php and yii

Question

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

Answer 1

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.

Answer 2

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).

Answer 3

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.

Answer 4

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.