Is my use of MVC overcomplicating things and cuasing duplicate code?

Question

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;
}

Answer 1

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

• In MVC, where do you draw the line between a controller and model? and
• Fat models, skinny controllers and the MVC design pattern.

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

• Rasmus Lerdorf's The No Framework Framework

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

Answer 2

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?

Answer 3

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.