SitePoint Sponsor

User Tag List

Page 2 of 3 FirstFirst 123 LastLast
Results 26 to 50 of 68

Thread: MVC vs PAC

  1. #26
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by TomB View Post
    Not at all, calling internal methods is fine
    Thank you for that acknowledgement. From post #20 onward, you seemed to hold the opposite opinion.
    "First make it work. Then make it better."

  2. #27
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    When you're using it to lock yourself into a specific configuration, I most certainly do hold the opposite opinion. As I said, there's times to use things and times to avoid them. Using $this->method() locks you into a very specific implementation, it's up to you as a developer to decide whether that's a problem or not. In the context of a method that you may want to substitute for something else, that is definitely a problem.

    edit: As a rule of thumb, if the contents of a method could be copy/pasted into the places it's called and the class would be just as useful it's fine. As well as if the method is public- if the outside world can use the object in this manner, using it internally will make no difference.

  3. #28
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    And what configuration did I lock myself into when I suggested the private method to consolidate repetitious code?

    EDIT:

    As a rule of thumb, if the contents of a method could be copy/pasted into the places it's called and the class would be just as useful it's fine.
    I'm pretty sure my private method suggestion was exactly such a case.
    "First make it work. Then make it better."

  4. #29
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Look at my Date example, it's analogous to this:


    PHP Code:
    class BlogController extends Controller
    {
        
    // ...

        
    private function renderList($items)
        {
            return 
    $this->render(
                
    'AcmeBlogBundle:Blog:list.html.php',
                array(
    'posts' => $items)
            );
        }

        private function 
    getPostsByUserId($userId)
        {
            
    $posts $this->get('doctrine')->getManager()
                ->
    createQuery('SELECT p FROM AcmeBlogBundle:Post p WHERE p.userId = ' $userId)
                ->
    execute();

            return 
    $posts;

            
    // A little further into the refactoring process,
            // and Symfony projects would actually move this data fetching code
            // into a PostRepository class
        
    }

    Here, renderList() can only ever use that very specific implementation. You cannot use a different template, for instance. getPostsByUserId() cannot be substituted at runtime either. The suggestion of a "PostRepository" would fix this, but you'd call that from the individual methods, not via the getPostsByUserId() method.

    The problem is that it's self-configuring. Your class cannot be configured in any other way because it's stateless.

  5. #30
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Quote Originally Posted by Jeff Mott View Post

    I'm pretty sure my private method suggestion was exactly such a case.
    That private method is indeed, but the private methods in your example code were not.

  6. #31
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by TomB View Post
    Here, renderList() can only ever use that very specific implementation. You cannot use a different template, for instance. getPostsByUserId() cannot be substituted at runtime either.
    Looking once again at your WireItUp code, you're also "locked" into a specific implementation. Your WireItUp hardcodes the template name (postList.tpl) as well as the data fetching logic (EveryPostList).
    "First make it work. Then make it better."

  7. #32
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    I think you misunderstand. That code could potentially supply anything at all because nothing is tightly coupled. It doens't matter where the configuration comes from, be it a static string passed directly or somewhere else. Calling $this->getSomeConfiguration() means that the configuration can only come from the getSomeConfigruation() method of the specified class. It's loose vs tight coupling.

  8. #33
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by TomB View Post
    ...That code could potentially supply anything at all...
    Indeed. Your WireItUp code can use whatever template it wants and it can instantiate whatever data fetching object it wants. Likewise, a framework controller can render whatever template it wants and fetch whatever data it wants.

    What am I missing?
    "First make it work. Then make it better."

  9. #34
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Ok, before we continue: back to basics, do you agree that the date class I provided, is not OO because it is stateless, it can only ever be configured in one very specific way and changing that configuration requires some very ugly workarounds?

    PHP Code:
    class Date {
        private function 
    getCurrentDate() {
            return 
    time();        
        }


        private function 
    format($date$format) {
            return 
    date($format$date);
        }


        public function 
    output() {
            return 
    $this->format($this->getCurrentDate(), 'd/m/Y');

        }

    The problem:

    -It's impossible to create a Date object which represents anything other than current date because the class configures itself.

    There is nothing here that makes use of OOP and it can equally be expressed with the code:

    PHP Code:
    function getCurrentDate() {
        return 
    time();        
    }


    function 
    format($date$format) {
        return 
    date($format$date);
    }

    function 
    output() {
        return 
    format(getCurrentDate(), 'd/m/Y');


    Can we agree on that?


    As an extension, the BlogController class shares this problem, it's not OO and its state is created from within.

    The problem:

    -It's impossible to configure the BlogController with different Views or Models because there is no real model or view. The state of the model and the view must always be supplied by the controller.


    As I said, this is an important point and unless you can see that, it's pointless to continue this debate


    The knock on effect of this is that the application is stateless. You cannot pass around stateful view or model objects to other view or model objects. They are procedural and need to be supplied with their state every time they're used. This goes back to my point about multiple instances of the same view with different states. When the view is just a template it needs to be supplied with its state every time which means means either repeated code or limited flexibility. This is identical to the difference between formatDate('d/m/Y', $timestamp); and $date->format('d/m/Y'). $this->renderView('template.tpl', $args); vs $view->render();

    If you can't understand or agree on that, I'm not sure I can continue this discussion. I've stripped it back to basics with the most trivial example possible.

  10. #35
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Would you feel better if BlogController had some state?

    Code:
    class BlogController
    {
        public function __construct($doctrine, $templating)
        {
            $this->doctrine = $doctrine;
            $this->templating = $templating;
        }
        
        public function listAction()
        {
            $posts = $this->doctrine->getManager()
                ->createQuery('SELECT p FROM AcmeBlogBundle:Post p')
                ->execute();
    
            return $this->templating->render(
                'AcmeBlogBundle:Blog:list.html.php',
                array('posts' => $posts)
            );
        }
    }
    "First make it work. Then make it better."

  11. #36
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Speaking of which... did you notice that your WireItUp code configures itself? The state of the model and the view is always supplied by your WireItUp code.

    You seem to frequently forget that your WireItUp code is analogous to framework controllers, and every "criticism" that you've lobbed against framework controllers applies equally to your WireItUp code.
    "First make it work. Then make it better."

  12. #37
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    But that's not the state, that's simply using doctrine/templating in order to set it's own state.

    The state here the controller is "what template is being used?" along with "what data is needed?"

    Whether it fires that state off to collaborators is irrelevant. You simply cannot use a different template in the listAction() method without repeating logic or restructuring the class after the fact. The state is always the same. Calling listAction() will always use the template 'AcmeBlogBundle:Blog:list.html.php' and always pass it the data from a query that runs: SELECT p FROM AcmeBlogBundle:Post p


    It is still self-configuring. To fix this by making the smallest possible alteration to your existing code, you'd do this:



    PHP Code:
    class BlogController
    {
        public function 
    __construct($doctrine$templating$template$query)
        {
            
    $this->doctrine $doctrine;
            
    $this->templating $templating;
        
    $this->query $query;
        
    $this->template $template;
        }
        
        public function 
    listAction()
        {
            
    $posts $this->doctrine->getManager()
                ->
    createQuery($this->query)
                ->
    execute();

            return 
    $this->templating->render(
                
    $this->template,
                array(
    'posts' => $posts)
            );
        }


    This is now a stateful object. Of course your models and views aren't stateful but it's a step in the right direction. Let's make the model and view stateful as well:


    PHP Code:
    class BlogController
    {
        public function 
    __construct($data$template)
        {
        
    $this->data $data;
            
    $this->template $template;        
        }
        
        public function 
    listAction()
        {
        return 
    $this->template->render($this->data);
        }



    This way, the controller could be constructed with any template and any data set and be reusable. It doesn't Dig into collaborators and it doesn't matter where the template comes from or where the data comes from. This creates a far more flexible application.


    The template object could be anything, it doesn't have to have loaded a .tpl.php file, it doesn't have to generate HTML.. it could generate a PDF, a CSV, a spreadsheet, etc and the data doesn't have to have come from a database. It could have come from a CSV, a web service, anywhere.

  13. #38
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by TomB View Post
    It is still self-configuring. ... The state here the controller is "what template is being used?" along with "what data is needed?"
    All of which also applies to your WireItUp code.

    You're trying to fix problems that don't exist, and you're turning a blind eye to the fact that these "problems" also exist in your own code.
    "First make it work. Then make it better."

  14. #39
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    Speaking of which... did you notice that your WireItUp code configures itself? The state of the model and the view is always supplied by your WireItUp code.

    You seem to frequently forget that your WireItUp code is analogous to framework controllers, and every "criticism" that you've lobbed against framework controllers applies equally to your WireItUp code.

    Once again, in any framework you need a router that does some decision making based on some kind of configuration. In your PAC framework, this is "This route initiates this controller and calls this action". All of that "WireItUp" code can be configured here. "This route initiates this view with this template and passes it this model".

    You need a router that contains configuration in either scenario. Choosing a view is no different to choosing a controller in this sense. Your argument is flawed because you're forgetting that you need some configuration somewhere. Your controller essentially duplicates this effort by having configuration spread across two different places.

  15. #40
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    If you honestly can't see the advantages of:


    PHP Code:
    class BlogController
    {
        public function 
    __construct($doctrine$templating$template$query)
        {
            
    $this->doctrine $doctrine;
            
    $this->templating $templating;
        
    $this->query $query;
        
    $this->template $template;
        }
        
        public function 
    listAction()
        {
            
    $posts $this->doctrine->getManager()
                ->
    createQuery($this->query)
                ->
    execute();

            return 
    $this->templating->render(
                
    $this->template,
                array(
    'posts' => $posts)
            );
        }


    Over


    PHP Code:
    class BlogController
    {
        public function 
    __construct($doctrine$templating)
        {
            
    $this->doctrine $doctrine;
            
    $this->templating $templating;
        }
        
        public function 
    listAction()
        {
            
    $posts $this->doctrine->getManager()
                ->
    createQuery('SELECT p FROM AcmeBlogBundle:Post p')
                ->
    execute();

            return 
    $this->templating->render(
                
    'AcmeBlogBundle:Blog:list.html.php',
                array(
    'posts' => $posts)
            );
        }


    Then I really can't continue this discussion because you don't understand the most basic OOP principles.

  16. #41
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    692
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    I guess you will be glad to know that either of your BlogController classes will work just fine in Symfony2. Picking a template based on the output type is trivial.

  17. #42
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Well that was trivial, my point really was that if whatever is initialsing that class knows enough to initialise the class it can be refactored to:

    PHP Code:
    class BlogController
    {
        public function 
    __construct($data$template)
        {
        
    $this->data $data;
            
    $this->template $template;        
        }
        
        public function 
    listAction()
        {
        return 
    $this->template->render($this->data);
        }

    And then, because there's nothing really happening, refactored further to:

    PHP Code:
    $template->render($data); 
    Skipping the class entirely.



    ....and I'm still waiting for an answer to "Why is PAC better than MVC?" The best I've seen so far is "Symfony can do that too" and in most of cases it can, but at the expense of either repeating code or limiting future flexibility. However you look at it, even if "Symfony can do that too" and its solution is equally as eloquent (which, as I've shown, it isn't!) it's still not a reason for choosing PAC over MVC.

  18. #43
    SitePoint Wizard bronze trophy Jeff Mott's Avatar
    Join Date
    Jul 2009
    Posts
    1,311
    Mentioned
    19 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by TomB View Post
    ...my point really was that if whatever is initialsing that class knows enough to initialise the class it can be refactored to: ... And then, because there's nothing really happening, refactored further to: ... Skipping the class entirely.
    And all the logic that used to be in the controller, you've now put in the router. That's not better. That's a bad solution to fix a non-problem.

    Quote Originally Posted by TomB View Post
    ....and I'm still waiting for an answer to "Why is PAC better than MVC?"
    See post #11.

    It comes down to the fact that if you code true MVC, then you're putting in extra effort to handle scenarios that will never happen in a web application.
    "First make it work. Then make it better."

  19. #44
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Quote Originally Posted by Jeff Mott View Post
    And all the logic that used to be in the controller, you've now put in the router. That's not better. That's a bad solution to fix a non-problem.

    For at least the third time, you need some configuration somewhere routing to a view is no different than routing to a controller. Why is it a "bad solution"? You call it a "non-problem".. which itself means that it doesn't need to be solved!

    Once again, it seems to be that you limiting yourself based on poor design decisions you've made higher up the application rather than applying proper OOP concepts. Your controller clearly breaks the Single responsibility principle and as such, encounters all the problems listed in Flaw: Class Does Too Much

    SRP and other similar principles aren't simply stylistic guides, they're there because they make the code more robust, maintainable and flexible.



    For the third time, in what scenario is:

    PHP Code:
    class BlogController
    {
        public function 
    __construct($doctrine$templating)
        {
            
    $this->doctrine $doctrine;
            
    $this->templating $templating;
        }
        
        public function 
    listAction()
        {
            
    $posts $this->doctrine->getManager()
                ->
    createQuery('SELECT p FROM AcmeBlogBundle:Post p')
                ->
    execute();

            return 
    $this->templating->render(
                
    'AcmeBlogBundle:Blog:list.html.php',
                array(
    'posts' => $posts)
            );
        }


    better than:


    PHP Code:
    class BlogController
    {
        public function 
    __construct($doctrine$templating$template$query)
        {
            
    $this->doctrine $doctrine;
            
    $this->templating $templating;
        
    $this->query $query;
        
    $this->template $template;
        }
        
        public function 
    listAction()
        {
            
    $posts $this->doctrine->getManager()
                ->
    createQuery($this->query)
                ->
    execute();

            return 
    $this->templating->render(
                
    $this->template,
                array(
    'posts' => $posts)
            );
        }



    See post #11.

    It comes down to the fact that if you code true MVC, then you're putting in extra effort to handle scenarios that will never happen in a web application.

    For at least the second time: As web developers the only difference between MVC and PAC is that the controller isn't a mediator between models and views, the view gets its own data from the model. This is it. This is the only difference. Explain to me exactly why the view should not have access to the model.

    And i suspect your answer is "My framework doesn't have a model" or "I don't understand what a model is".

    And again, for at least the third time: Your application needs to know something at the entry point. Whether you're creating a "controller" (Which fits no known definition of a controller outside the PHP world) or a view is irrelevant. Something must be created with some parameters be it a view or a "controller". This is a non-issue and one you're inventing to avoid the real question of "why shouldn't a view get its own data from the model?"


    And once again: Why is PAC better than MVC? You still haven't answered this very simple question without drifting off into territories which are well beyond the scope of MVC and PAC. The topic of application entry points falls beyond this, either way the application needs to be initialised, it has zero bearing on MVC vs PAC and is a separate issue.

  20. #45
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    692
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    Perhaps we could look at a different issue? The question of forms has come up several times. Here is an example of editing an item in S2:
    PHP Code:
    class GameController extends Controller
    {
        public function 
    editAction(Request $request$gameId)
        {
            
    $manager $this->('cerad_arbiter.game.manager');
            
            
    $game $manager->loadGameForId($gameId);

            
    // Build the form
            
    $formType $this->get('cerad_arbiter.game_edit.formtype');
            
            
    // The form itself
            
    $form $this->createForm($formType,$game);
            
            if (
    $request->getMethod() == 'POST')
            {
                
    $form->bindRequest($request);

                if (
    $form->isValid())
                {
                    
    $manager->flush();
                    
                    return 
    $this->redirect($this->generateUrl('cerad_arbiter_game_edit'));
                }
            }
           
            
    $tplData = array();
            
    $tplData['game'] = $game;
            
    $tplData['form'] = $form->createView();
            return 
    $this->render('CeradArbiterBundle:Game:edit.html.twig',$tplData);
        }

    How would using your version of MVC improve this?

  21. #46
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    Can you explain what a "Manager" is in this context? It suffers from Flaw: Digging into collaborators so it's impossible for me as a third party to know what this code is doing. I'll assume it's loading a $game object which it's using to populate fields on the form but you'll have to correct me if I'm wrong.

    For how MVC would improve this, it would add a proper separation of concerns and allow for far greater flexibility. In your example there the following are all tightly coupled: The form object being created, where the data being loaded into the form is retrieved from (it has to come from a very specific manager implementation) and the view (Like the BlogController example above, it's impossible to reuse this code with a different view because the template name is hardcoded). Essentially, the model, view and controller. They're all very tightly coupled.

    How are you processing the result? In this code I'd expect to see some logic inside the isValid() block that saved form data back into the model when the form was submitted and valid. Is that what $manager->flush() does? If so that's an incredibly poor name for something which modifies persistent state.


    I think it was helpful before when I showed how to make the code more flexible with the smallest alterations I'll do what I did above and using the fewest possible changes to your example code refactor it to add flexibility:


    PHP Code:
    class GameController extends Controller 


        public function 
    __construct($manager$formType$redirect$template) {
            
    $this->manager $manager;
            
    $this->formType $formType;
            
    $this->redirect $redirect;
            
    $this->template $template;

        }

        public function 
    editAction(Request $request$gameId
        { 
            
    $manager $this->($this->manager); 
             
            
    $game $manager->loadGameForId($gameId); 

            
    // Build the form 
            
    $formType $this->get($this->formType); 
             
            
    // The form itself 
            
    $form $this->createForm($formType,$game); 
             
            if (
    $request->getMethod() == 'POST'
            { 
                
    $form->bindRequest($request); 

                if (
    $form->isValid()) 
                { 
                    
    $manager->flush(); 
                     
                    return 
    $this->redirect($this->generateUrl($this->redirect)); 
                } 
            } 
            
            
    $tplData = array(); 
            
    $tplData['game'] = $game
            
    $tplData['form'] = $form->createView(); 
            return 
    $this->render($this->template,$tplData); 
        } 


    Can you see why that's better? Because this is the building blocks for understanding separation of concerns and reusable code, understanding why this is an improvement in terms of flexibility is a fundamental step towards writing flexible code. Essentially, a GameController is now fully configurable. Multiple instances of GameController can be initiated with different configurations. This allows re-using the code and swapping in/out any of the components without repeating any code at all or adding new actions which would need to repeat a lot of the logic. Using your existing code with a different template means adding a new controller action which repeats all the logic apart from one line.

    I'll go a little bit further and refactor it even more to reduce the code and add flexibility:


    PHP Code:
    class GameController extends Controller 


        public function 
    __construct(Manager $managerFormType $formTypeGeneratedUrl $redirect$template) {
            
    $this->manager $manager;
            
    $this->formType $formType;
            
    $this->redirect $redirect;
            
    $this->template $template;

        }

        public function 
    editAction(Request $request$gameId
        { 
             
            
    $game $this->manager->loadGameForId($gameId); 
            
            
    // The form itself 
            
    $form $this->createForm($this->formType,$game); 
             
            if (
    $request->getMethod() == 'POST'
            { 
                
    $form->bindRequest($request); 

                if (
    $form->isValid()) 
                { 
                    
    $this->manager->flush(); 
                     
                    return 
    $this->redirect($this->redirect); 
                } 
            } 
            
            
    $tplData = array(); 
            
    $tplData['game'] = $game
            
    $tplData['form'] = $form->createView(); 
            return 
    $this->render($this->template,$tplData); 
        } 


    Can you see where this is heading? I'm refactoring out any code which is irrelevant because fully constructed objects can be passed into the constructor instead of supplying that logic in the controller itself. This means the controller is now apathetic towards where its dependencies come from. This immediately adds flexibility because, for example working out $formType does not have to happen in the GameController::get() method. It can happen absolutely anywhere, and it can be configured differently for every instance of the GameController object that you may ever use.

  22. #47
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    -- Sitepoint is erroring tonight when I attempt to make large posts, I've had to split this up --


    Now, on to MVC proper. Again, I don't understand what the ambiguously named "Manager" is doing so it's very difficult to replicate this code but here's how I'd handle forms in MVC ( In an effort to keep things simplistic, these are all examples cut down to their bare minimum for demonstration, so it likely presents some obvious "But how would you..." questions when dealing with variations on this)



    Firstly, a generic, reusable view class that contains all the display logic that's used for any form:

    PHP Code:
    class FormView implements View {
        private 
    $model;
        private 
    $template;

        public function 
    __construct(FormViewModel $modelTemplate $template) {
            
    $this->model $model;
        }

        public function 
    output() {
            
    $form $this->model->getForm();
            
            if (!
    $form->isValid())  {
                
    $this->template->assign('form'$form);
                
    $this->template->assign('errors'$form->getErrors());
                return 
    $this->template->output();
            }
            else {
                return 
    $this->model->getSubmittedView()->output();
            }        

        }

    And an interface for it to use:


    PHP Code:
    interface FormViewModel {
        public function 
    getForm();
        public function 
    getSubmittedView();
        public function 
    isValid();

    Finally, also reusable and probably provided by the framework, a Model which will work in most use-cases but can be extended or completely replaced with another class that implements FormViewModel if needed:


    PHP Code:
    //Extensible, reusable FormViewModel which covers 90%+ of use-cases.

    class GenericFormViewModel implements FormViewModel {
        private 
    $form;
        private 
    $request;
        private 
    $data;


        public function 
    __construct(Form $formView $submittedViewRequest $request) {
            
    $this->form $form;
            
    $this->submittedView $submittedView;
            
    $this->request $request;
        }

        public function 
    getSubmittedView() {
            
    //This could potentially have some logic depending on what's been submitted
            //By providing an accessor here, the FormView class has no knowledge of where this comes from
            //It's changable at runtime

            //This could be a URL to redirect to instead of a view to show after submission, but that's another topic entirely
            
    return $this->submittedView();
        }

        public function 
    getForm() {
            if (!empty(
    $this->data)) $this->form->populate($this->data);    
            
    $this->form->populate($this->request);
            
            return 
    $this->form;    
        }

        public function 
    isValid() {
            
    //The form object itself should be generic and only handle simple validation, required fields, regex cheks, etc.
            // Things that required domain logic such as 
            //"That email address is already registered" need to happen here at the model layer, not the Form (View) layer
            
    return $this->getForm()->isValid();
        }

        public function 
    setData($data) {
            
    $this->data $data;
        }


  23. #48
    SitePoint Guru bronze trophy TomB's Avatar
    Join Date
    Oct 2005
    Location
    Milton Keynes, UK
    Posts
    996
    Mentioned
    9 Post(s)
    Tagged
    2 Thread(s)
    -- Sitepoint is erroring tonight when I attempt to make large posts, I've had to split this up --


    Again, this would be provided by the framework itself. This seems comparatively like significantly more code than the example you've shown above, but you have left out a very large amount of code that would have been provided by the framework for a lot of the tasks there.


    Finally, in most cases the end-developer would only provide the controller:


    PHP Code:
    class GameFormController {
        private 
    $model;
        private 
    $request;
        private 
    $game;

        public function 
    __construct(FormViewModel $modelGame $gameRequest $request) {
            
    $this->model $model;
            
    $this->game $game;
            
    $this->request $request;
        }    

        public function 
    edit($id) {
            
    $this->model->setData($this->game->findById($id));
        }

        public function 
    save() {
            if (
    $this->model->isValid())    {
                
    $this->game->save($this->request);
            }
        }



    The advantages here?

    - Everything that applies to the BlogController above, it's not self-configuring, it adheres to the Single Responsibility Principle and as such is far more flexible. I can reuse any of the code here and replace any of the components, I can use a different form, a different template or completely change the logic of how the form is populated without needing to repeat existing code because my objects are following SRP, the most basic parts can be substituted at runtime.

    - Because there's a better separation of concerns, tasks common to any form such as assigning errors to the view and populating the view based on a model or $_POST have been abstracted away and are reusable.

    - This handles editing or adding. In most cases the only difference between "Add" and "Edit" is whether or not the form is pre-filled. Your code above needs a separate controller action for "Add" which repeats a lot of code. Here, the router just routes to the controller's edit() method and the form gets populated. If edit() is never called, the form isn't populated and can act as it's adding a new record. And because everything is separated out, edit/add can even very easily use different templates but retain everything else identically.

    - For most forms, there will be very little code provided by the application developer, they'd supply a controller as above and the Form Object. The old OOP adage "programming by difference" applies here.

    - As we know, Most if-statements are evil. By using polymorphism to handle program flow where possible instead of if statements gives us the ability to change the conditions at runtime by passing in different objects. The controller shouldn't really care whether the form is using $_GET or $_POST, that should be encapsulated within the form.

  24. #49
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    692
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    Let me start by thanking you for your detailed answer. I'll try to respond to different parts as time permits. Oddly enough I lost internet connection last night just as I was reading your stuff.

    Can you explain what a "Manager" is in this context?
    For manager you can think application model. It's actually a Doctrine 2 ORM Repository with some application specific methods added. As you suspected, $manager->loadGameForId() does indeed load a game object.

    Note that the game object itself is really a object graph and contains other objects such as teams, location etc. The $manager->flush() persists any changes in the graph to the actual database. I should also point out that Doctrine is not an active record system so there is no $game->save() method.

    $form->isValid() processors any validators associated with the game object. If isValid fails then error messages are automatically stored in the form view for displaying.

  25. #50
    SitePoint Guru
    Join Date
    Nov 2003
    Location
    Huntsville AL
    Posts
    692
    Mentioned
    4 Post(s)
    Tagged
    1 Thread(s)
    I don't mean to take you out of context but I'll admit that this snippet concerns me:
    but here's how I'd handle forms in MVC
    It is the "I'd handle" part that raises a flag. I would have expected "I handle". In other words, I'd would like to see code (possibly stripped down) that you use in your actual production MVC system.

    Do you actually have a FormViewModelInterface and do you actually have a GenericFormViewModel class? If so, I'm wondering if you could post the actual code, warts and all?

    And if you don't actually have the above classes in production then would you mind posting what you actually use? Better yet would be a link to your actual MVC framework code.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •