tags:

views:

86

answers:

3

I've wrote my own PHP MVC however i'm struggling to distinguish between the model and controller part. For example with a simple form that will add data to the database, the controller is pulling the form data from a request object, and passing it to the Model to handle the actual database insert. However there seems a lot of somewhat duplicate code in here, would it not be simpler to remove the Model part altogether and simply have the controller do the database insert?

I understand that in some cases i may have multiple controllers using the same Model action, however for the occasioanl time this happens it doesn't seem worth all the extra coding required to constantly seperate the Model and controller?

It's not so much duplicate code more that it seems a long winded way of doing things as i'm writing a lot of code for what is essentially a simple function, if that makes sense?

Sample code from controller

// processes the new site data
public function add_new_process() {
    // execute action in model
    $Model_Websites = new Model_Websites();
    $name           = $this->request->getPropertyFiltered('sitename',array('sanitize'));
    $descrip        = $this->request->getPropertyFiltered('descrip',array('sanitize'));
    $url                = $this->request->getPropertyFiltered('siteurl',array('sanitize'));
    $signup_url = $this->request->getPropertyFiltered('signupurl',array('sanitize'));
    $acct_id    = $this->request->getPropertyFiltered('acct_id',array('sanitize'));
    $thumbnail  = $this->request->getPropertyFiltered('thumb',array('sanitize'));
    if($Model_Websites->addNewSite($name,$descrip,$url,$signup_url,$acct_id,$thumbnail)) {
        $this->request->addFeedback("Your new website has been added succesfully!");
        $this->request->setFeedbackStatus(true);
        $this->request->storeFeedbackInSession();
        $this->template->redirectBrowser(__SITE_URL.'/websites/');
    } else {
        $this->template->setProperty('page_title', Registry::getConfig('site_name').' :: Add New Website' );
        $this->template->render('websites','show_form'); // controller,view
    }
}

Sample code from Model

function addNewSite($name,$descrip,$url,$signup_url,$acct_id,$thumbnail) {
    $pdo = ConnectionFactory::instance()->getConnection();
    $stmt = $pdo->prepare("INSERT INTO {$this->db_table_websites} SET 
                name = :name
            , descrip = :descrip
            , url = :url
            , signup_url = :signup_url
            , acct_id = :ccbill_site_id
            , thumbnail = :thumbnail
            ");
    $stmt->bindParam(':name', $name, PDO::PARAM_STR);
    $stmt->bindParam(':descrip', $descrip, PDO::PARAM_STR);
    $stmt->bindParam(':url', $url, PDO::PARAM_STR);
    $stmt->bindParam(':signup_url', $signup_url, PDO::PARAM_STR);
    $stmt->bindParam(':acct_id', $acct_id, PDO::PARAM_STR);
    $stmt->bindParam(':thumbnail', $thumbnail, PDO::PARAM_STR);
    if($stmt->execute()) return true;
        else return false;
}
A: 

Your Controller do all data manipulations between the View and the Model. This could be parsing data, maths manipulations and so on as required. The model just saves/update/delete records. So no don't remove neither Controller or Model. Just try not to mix the tasks between layers.

infinity
+2  A: 

I am not trying to be rude, so please don't take my comments incorrectly...

However, if you are struggling with the difference between what the responsibilities are between the model and the controller, perhaps MVC isn't the right answer for the problems you are trying to solve. MVC is just a pattern, and it isn't necessarily right in every situation.

But, in answer to your question...

The Model represents the objects your application is dealing with. Things like an Order, Customer, and Invoice are examples of models in an e-commerce application. MVC coupled with an Active Record pattern also implies that the models persist and retrieve themselves from the database.

The Controller is the conductor of this three ring circus. It is the controller's responsibility to talk to the models, and send them appropriate messages (like save, delete, etc).

The View is responsable for taking data (i.e., Models) that the Controller has fetched, created, etc and displaying them. The view can render to HTML, JSON, or anything else.

And, as a closing comment... are there not already MVC frameworks written for PHP?

Jason Whitehorn
Yes you're probably right, if i was too do it again i would likely not use a MVC pattern as it's a fairly simple project... What can i cay, boredom and an inquisitive mind are a dangerous combo! I did take a look at the premade frameworks but they all seem somewhat bulky for my needs.
IainW
+2  A: 

Well, the question is whether you do want to use MVC or not. If you want to use MVC, then you should not put business logic into the controller, because that's not where it should be. Please go through

However, no one forces you use MVC. It's a common pattern and it is good practise to use it when you want to create a maintainable application. Especially the separation of business logic and presentation layer make it worth considering. But with small applications and websites, chances are MVC is oversized. You could just as well structure your site with a bunch of Transaction scripts, where each script handles a single request from the UI. Check out

for a possible alternate approach.


As for your code, I dont think it's overcomplicating things. It probably looks just like it was, because of the verbose code. You could streamline it a bit by creating a FilterChain (alternative) that sanitizes all input transparently before your controller is called. And you could make your form use grouping so you can just pass $form['site'] to your Model with the other values being subkeys of that. Also, you are doing three calls to set the feedback that could probably be handled in one call. Maybe you could write a Feedback Helper to do the three calls for you, but that only exposes one method and does the remaining work internally (or make it accept three arguments or whatever is necessary to cut down on the work needed to add a feedback message).

Gordon
Thanks, some most usefull links there. I'm begining to realise that perhaps a MVC was overkill for this particular project and it would likely have been simpler with just a set of basic transaction scripts as you mention. :)
IainW
+1 for link to Rasmus Lerdorf's post.
BoltClock
Yes i think part of the problem was that i scared myself when looking at the amount of code. Your suggestions for trimming the fat certainly make sense, i'm already using filter_input as part of the sanitize function but as you suggest, it would make more sense to do this globaly in the request class instead of for each individual item. My feedback functions could definately do with some cleaning up too! Thanks, you've made me feel a lot more comfortable about going forwards with this project in its current form :)
IainW
@IainW you're welcome :)
Gordon