SitePoint Sponsor

User Tag List

Page 1 of 3 123 LastLast
Results 1 to 25 of 52

Thread: Pragmatic MVC

  1. #1
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Pragmatic MVC

    Hey there,

    I've read the sticky at the top of this subforum, tons of good information there. I've also read some of Harry's posts that have gone a long way in helping me see how MVC can be implemented.

    I'm not really confused, but I just wanted to share what I've got so far to see what you guys think. The script installs a database then outputs any error messages.

    PHP Code:
    <?php

    require_once 'views/base_html_view.php';
    require_once 
    'views/schema_view.php';
    require_once 
    'libs/classes/Database.php';
    require_once 
    'models/schema_model.php';


    class 
    SchemaController
    {

        public 
    $SchemaView;

        function 
    __construct()
        {
            
    $SchemaModel = new SchemaModel();
            if (isset(
    $_GET['plaintext']))
            {
                    
    $this->SchemaView = new SchemaPlaintextView($SchemaModel);
            } 
            
            else 
            {
                    
    $this->SchemaView = new SchemaHTMLView($SchemaModel);
            }
            
        }
        
    }

    $SchemaController = new SchemaController();
    $SchemaController->SchemaView->SchemaModel->createUserTable();
    $SchemaController->SchemaView->display_errors();

    echo 
    $SchemaController->SchemaView->toString();

    ?>
    I adopted a pragmatic approach when deciding how I should implement MVC; I focused on what it aides, as you can see above I can easily switch the output from HTML to plaintext. That's pretty cool in itself, but are there any other cool things I can do?

    Is the above implementation along the right lines?

  2. #2
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi,

    PHP Code:
            if (isset($_GET['plaintext']))
            {
                    
    $this->SchemaView = new SchemaPlaintextView($SchemaModel);
            } 
    using $_GET is on the lines of using global variables. Global variables in class names is considered to be a bad thing, you should pass it in the __construct() method as a parameter instead.

  3. #3
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    /\ But that would make it more complex. KISS. Don't do what others do, do what *you* think is good. Inspiration is good, of course, but adoption without considering advantages/drawbacks...


    Your method is pretty pragmatic, but there still is boilerplate code. If I do MVC in PHP I create database functions (or a class if you prefer). Then I create a directory for each controller and a file for each action (method). The visitor visits these files. An action file might look like this:

    PHP Code:
    <?php
    include 'models/posts.php';

    $posts post_list(); # SELECT * FROM posts

    include 'views/post_list.php';
    ?>
    and post_list.php looks like:

    PHP Code:
    <ul>
    <? foreach($post in $posts){ ?>
    <li><?= $post['title'?></li>
    <? ?>
    </ul>
    So your example would look like this (if you define a few helper functions like require_model and display):

    PHP Code:
    <?php

    require_model
    ('schema');

    $SchemaModel = new SchemaModel();
    $SchemaModel->createUserTable();

    if(isset(
    $_GET['plaintext'])){
      
    display('schema/plaintext');
    }else{
      
    display('schema/html');
    }

    ?>
    I prefer non-oo if possible because you'll need less code and runs faster, but the end result is the same of course. The possibility to change the view is cool, and you can change a model in the same way to use an xml database for example (as long as the interface of the model is the same). This is much harder if you hardcode sql in views.

    The best thing about MVC though, is that you keep your head tidy: you don't have to think about everything at once, but just one aspect (M, V or C).

  4. #4
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    How would it make it more complex? It's just moving into the constructor parameter and using that instead. I'm not sure what makes that "complex".

  5. #5
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    By adding an extra parameter to the constructor, that is essentially $_GET, you make it more complex because you have to think "O, this is $_GET". You need an extra step in your brain, and you need to type it over and over again. There is really no benefit. Most times globals are evil, but I don't think this is the case with $_GET.

  6. #6
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Your logic is still failing me:

    PHP Code:
        function __construct($html true)
        {
            
    $SchemaModel = new SchemaModel();
            if (
    $html)
            {
                    
    $this->SchemaView = new SchemaHTMLView($SchemaModel);
            }
            else
            {
                    
    $this->SchemaView = new SchemaPlaintextView($SchemaModel);
            } 
    heck, if you wanted to even expand to more output types:

    PHP Code:
        function __construct($driver 'html')
        {
            
    $SchemaModel = new SchemaModel();
            
    select ($driver)
            {
                    case 
    'html':
                       
    $this->SchemaView = new SchemaHTMLView($SchemaModel);
                       break;
                     case 
    'text':
                       
    $this->SchemaView = new SchemaPlaintextView($SchemaModel);
                        break;
            } 
    and you just add to case list whenever you want to have a new output class handler.

  7. #7
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I think you meant switch instead of select.

    This is more complex because there is an extra variable, and you have to pass it in. Also, if you want to accept more parameters, you have to change your code in *three* places: the code that passes the get into the controller, the parameter list, and in the actual code. If you just use $_GET you have to change the actual code only.

    and you just add to case list whenever you want to have a new output class handler.
    This has nothing to do with $_GET.

  8. #8
    SitePoint Wizard chris_fuel's Avatar
    Join Date
    May 2006
    Location
    Ventura, CA
    Posts
    2,750
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Also, if you want to accept more parameters, you have to change your code in *three* places: the code that passes the get into the controller, the parameter list, and in the actual code.
    You have to do that anyways regardless of how many parameters there are. I guess the question comes down to why is $_GET being used in the first place. Something seems weird.

  9. #9
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    > I guess the question comes down to why is $_GET being used in the first place.

    Why indeed - you do not want to have a direct reference to $_GET et al it's self as you lose encapsulation; That is why myself, and a lot of regular members use an object instead, used to access $_GET et al.

    Just say for example, that you want to filter $_GET et al, so how would you apply those filters for example? You couldn't, thus you are better to encapsulate $_GET et al within an object (ie Request) where you could better manage it

  10. #10
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I took on board what you said chris_fuel, I changed it to this:

    PHP Code:
    class SchemaController
    {

        public 
    $SchemaView;

        function 
    __construct($driver)
        {
            
    $SchemaModel = new SchemaModel();
            
            switch (
    $driver)
            {            
                case 
    'plaintext':
                        
    $this->SchemaView = new SchemaPlainTextView($SchemaModel);
                break;
                
                default:
                        
    $this->SchemaView = new SchemaHTMLView($SchemaModel);
            }
            
        }
    }

    isset(
    $_GET['display']) ? $driver $_GET['display'] : $driver NULL// because of notice error
    $SchemaController = new SchemaController($driver);
    $SchemaController->SchemaView->SchemaModel->createUserTable();
    $SchemaController->SchemaView->display_errors();

    echo 
    $SchemaController->SchemaView->toString(); 
    I get what you mean about why the $_GET seems unnecessary in an install script (who's going to need to install using plaintext only right?) I was only meaning to experiment with this first bit of my application, since I build the database before I start coding, so this is all I have.

    Quote Originally Posted by chris_fuel
    using $_GET is on the lines of using global variables. Global variables in class names is considered to be a bad thing, you should pass it in the __construct() method as a parameter instead.
    Aha, yeah, I thought to myself after reading that, "What if I were to split the class and the implementation up?" Bam, I wouldn't know how the class was getting the information for the page view.

    Cheers for that one.

    So, apart from that, I'm along the right lines?

  11. #11
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Dr Livingston
    > I guess the question comes down to why is $_GET being used in the first place.

    Why indeed - you do not want to have a direct reference to $_GET et al it's self as you lose encapsulation; That is why myself, and a lot of regular members use an object instead, used to access $_GET et al.

    Aha, I remember reading a discussion about an object that maps all the globals into an object. After a little searching I found it.

    I could remember the recursive strip slashes from some php.net comments , I was particular interested in the way it implements part of the SPL: I didn't even know there was an SPL!

    If it's not trouble, would you elaborate on the pros of using that class? I'm still in a pragmatic mind and to me accessing a superglobal seems quick'n'easy.

  12. #12
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Your class SchemaController looks more like a factory to me. The important part of the controller code is in the few lines in the global scope, right after SchemaController.
    Why don't you stick thoose in a method on the class ? You could call it execute or something like that.
    The switching on GET looks a little peculiar to me - I would probably not do that in the constructor. It loks like something that fits in a factory-method, which can then be called by the execute method.
    You might want to return the rendered view from the controller, rather then echo'ing it out.

  13. #13
    SitePoint Member
    Join Date
    May 2006
    Location
    Saratov, Russia
    Posts
    13
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Hal9k
    PHP Code:
    $SchemaController = new SchemaController();
    $SchemaController->SchemaView->SchemaModel->createUserTable();
    $SchemaController->SchemaView->display_errors();

    echo 
    $SchemaController->SchemaView->toString(); 
    I think it's a responsibility of the SchemaController class rather than the main script to call the createUserTable method. So I'd suggest to move the method call into the controller:

    PHP Code:
    class SchemaController
    {
        public 
    $SchemaView;

        function 
    __construct()
        {
            
    // ...
        
    }

        function 
    run()
        {
            
    $this->SchemaView->SchemaModel->createUserTable();
        }    
    }

    $SchemaController = new SchemaController();
    $SchemaController->run();
    $SchemaController->SchemaView->display_errors();

    echo 
    $SchemaController->SchemaView->toString(); 
    There's another question: why is a view member being used to update the model? Maybe it's better to store the model being updated in the controller:

    PHP Code:
    class SchemaController
    {
        private 
    $SchemaModel;
        public 
    $SchemaView;

        function 
    __construct()
        {
            
    $this->SchemaModel = new SchemaModel;
            
    // ...
        
    }

        function 
    run()
        {
            
    $this->SchemaModel->createUserTable();
        }    


  14. #14
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks for the input ArtGor and kyberfabrikken. ArtGor, I agree I don't like the way the view updates the model, I really need to post some snippets from my Model and View to show why I did so, I'm on a public computer now, so I'll have to post some when I get back.

    Quote Originally Posted by kyberfabrikken
    You might want to return the rendered view from the controller, rather then echo'ing it out.
    Is this the done thing to do? I was under the impression that it was wrong to echo from the class itself?

  15. #15
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Hal9k
    Is this the done thing to do? I was under the impression that it was wrong to echo from the class itself?
    That's what I meant - return the string, and echo from the top level script instead of inside the class. You're doing that in the initial code - I just meant that if you took the global codeblock and put it in a function, you should still leave the echo statement outside the class.

  16. #16
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You have to do that anyways regardless of how many parameters there are. I guess the question comes down to why is $_GET being used in the first place. Something seems weird.
    $_GET is being used to get GET parameters ;-). You don't have to change your code in three places if you just use $_GET. Suppose you have this code:

    PHP Code:
    <?php

    class FooController
    {
        function 
    __construct($a)
        {
            
    # do something with $a
        
    }
    }

    $SchemaController = new SchemaController($_GET['a']);
    echo 
    $SchemaController->SchemaView->toString(); 

    ?>
    Now you want to use $_GET['b'] too:

    PHP Code:
    <?php

    class FooController
    {
        function 
    __construct($a$b)
        {
            
    # do something with $a
            # do something with $b
        
    }
    }

    $SchemaController = new SchemaController($_GET['a'], $_GET['b']);
    echo 
    $SchemaController->SchemaView->toString(); 

    ?>
    The code has to be changed in three places: in the parameter list: __construct($a, $b), in the actual code: # do something with $b and in the call to __construct: new SchemaController($_GET['a'], $_GET['b']);

    If you use $_GET you don't have this problem:

    PHP Code:
    <?php

    class FooController
    {
        function 
    __construct()
        {
            
    # do something with $_GET['a']
        
    }
    }

    $SchemaController = new SchemaController();
    echo 
    $SchemaController->SchemaView->toString(); 

    ?>
    =>

    PHP Code:
    <?php

    class FooController
    {
        function 
    __construct()
        {
            
    # do something with $_GET['a']
            # do something with $_GET['b']
        
    }
    }

    $SchemaController = new SchemaController();
    echo 
    $SchemaController->SchemaView->toString(); 

    ?>
    Just one change.

  17. #17
    eschew sesquipedalians silver trophy sweatje's Avatar
    Join Date
    Jun 2003
    Location
    Iowa, USA
    Posts
    3,749
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Fenrir2
    $_GET is being used to get GET parameters ;-). You don't have to change your code in three places if you just use $_GET. Suppose you have this code:
    ...
    PHP Code:
    $SchemaController = new SchemaController($_GET['a'], $_GET['b']);
    echo 
    $SchemaController->SchemaView->toString(); 
    ...
    Just one change.
    Why not
    PHP Code:
    $SchemaController = new SchemaController($_GET);
    echo 
    $SchemaController->SchemaView->toString(); 
    or
    PHP Code:
    $SchemaController = new SchemaController(new RequestWraper($_GET));
    echo 
    $SchemaController->SchemaView->toString(); 
    And you are back to the same "Just one change" but with more flexibility and less coupling.
    Jason Sweat ZCE - jsweat_php@yahoo.com
    Book: PHP Patterns
    Good Stuff: SimpleTest PHPUnit FireFox ADOdb YUI
    Detestable (adjective): software that isn't testable.

  18. #18
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    In your first example you're just renaming $_GET to some other variable. That is just extra work for nothing.

    The second example is the same, but it is even more work because you have to write RequestWrapper. I don't see how that would be more flexible/less coupling.

  19. #19
    Put your best practices away. The New Guy's Avatar
    Join Date
    Sep 2002
    Location
    Canada
    Posts
    2,087
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    What happens if you change your URL design and $_GET['view'] is no longer the view? And you couldn't get the view in that way?

    http://blah.com/controller/view/para...param2/value2/

    $_GET['view'] will no longer work and you have $_GET['view'] sprinked all over your code. It would be better to have a request (I would even put a url helper/handler over that too) to make sure things stay the same if you change the your URL design.

    Using get is tight coupling for that reason, it doesn't allow for the flexibility of design changes.
    "A nerd who gets contacts
    and a trendy hair cut is still a nerd"

    - Stephen Colbert on Apple Users

  20. #20
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by The New Guy
    Using get is tight coupling for that reason, it doesn't allow for the flexibility of design changes.
    That helped into understanding why I should use a class that maps out the superglobals.

    Just got to post some my other code to give the full picture now...

  21. #21
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    $_GET['view'] = $_GET['new_view'] ;-)

    Overdesign...

  22. #22
    Put your best practices away. The New Guy's Avatar
    Join Date
    Sep 2002
    Location
    Canada
    Posts
    2,087
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Fenrir2
    $_GET['view'] = $_GET['new_view'] ;-)

    Overdesign...
    No that wont work either, because in my url example you would $_GET['param'] and it would return something like "controller/view/param1/value1/param2/value2/" from which you would have to break it up by the delimiter "/".
    "A nerd who gets contacts
    and a trendy hair cut is still a nerd"

    - Stephen Colbert on Apple Users

  23. #23
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

  24. #24
    SitePoint Guru
    Join Date
    Aug 2005
    Posts
    986
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    No that wont work either, because in my url example you would $_GET['param'] and it would return something like "controller/view/param1/value1/param2/value2/" from which you would have to break it up by the delimiter "/".
    Yes, but you have to do exactly the same if your wrap it in a RequestWrapper.

    From the wikipedia page kyberfabrikken posted:
    However, in a few cases, global variables can be suitable for use. They can be used to avoid having to pass frequently-used variables continuously throughout several functions, for example.

  25. #25
    Afraid I can't do that Dave Hal9k's Avatar
    Join Date
    Mar 2004
    Location
    East Anglia, England.
    Posts
    640
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Here's the full picture:

    Directory structure:

    /install.php
    /views/schema_view.php
    /models/schema_model.php

    /views/shema_view.php:
    PHP Code:
    class SchemaHTMLView extends HTMLView
    {
        
        public 
    $SchemaModel;
        
        function 
    __construct($SchemaModel)
        {
            
    HTMLView::__construct('Title''style');
            
    $this->SchemaModel $SchemaModel;
        }
        
        function 
    display_errors()
        {
            if (
    $this->SchemaModel->checkError())
            {
                
    $Exception $this->SchemaModel->Exception->getMessageArray();
                
    $this->page .= "<p>" $Exception['database_reports'] . "</p>";
            }
        }
        
        function 
    toString()
        {
            
    $this->addFooter();
            return 
    $this->page;
        }
                
    }

    class 
    SchemaPlainTextView
    {
        
        public 
    $SchemaModel;
        private 
    $page;
        
        function 
    __construct($SchemaModel)
        {
            
    $this->SchemaModel $SchemaModel;
            
    $this->display_errors();
        }
        
        function 
    display_errors()
        {
            if (
    $this->SchemaModel->checkError())
            {
                
    $Exception $this->SchemaModel->Exception->getMessageArray();
                
    $this->page .= $Exception['database_reports'];
            }
        }
        
        function 
    toString()
        {
            return 
    $this->page;
        }
        
    }
            

    ?> 
    /models/schema_model.php:

    PHP Code:
    <?php


    class SchemaModel
    {
        
        private 
    $Database;
        public 
    $Exception;
        
        function 
    __construct()
        {
            try
            {
                
    $this->Database = new Database('server''root''password''database');
            }
            catch (
    DatabaseException $Exception)
            {
                
    $this->Exception $Exception;
            }
        }
        
        function 
    createUserTable()
        {
    // SQL goes here 
            
    try 
            {
                
    $this->Database->query($users);
            }
            catch (
    DatabaseException $Exception)
            {
                
    $this->Exception $Exception;
            }
        }
        
        function 
    checkError()
        {
            if (
    $this->Exception instanceof DatabaseException
            {
                return 
    True;
            }
            else 
            {
                return 
    False;
            }
        }
    }

    ?>
    My install.php is the controller I envisaged. Be as critical as you like with the code, it's my first foray into MVC, so I'm not pretending it's great. The install.php file is what I posted earlier.

    Also feel free to recommend any tutorials (I have been working off this) as I appreciate wading through that code might not be fun...


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
  •