SitePoint Sponsor

User Tag List

Results 1 to 15 of 15
  1. #1
    SitePoint Addict
    Join Date
    Mar 2002
    Location
    Los Angeles
    Posts
    325
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Criticize My Class Diagram/Structure

    I want everybody to criticise the crap out of my class diagram for this app I'm building. Here's the link to my class diagram:

    http://www.visible-form.com/blog/pos...es/k2_uml.html

    The layout may look a little funky becasue i was trying to squeeze everything into the screenshot, but here's the rundown:

    I have a SiteManager class that acts as a psuedo-controller, all the requests go through that.

    I have two main superclasses, DynamicOutputter and RecordHandler. Each section of the web site has a corresponding "outputter" class of its own...so for news it would be News_Outputter, and that inherits from DynamicOutputter. The outputters return the formatted output for the page. Each outputter is composed with its corresponding RecordHandler(same deal for the RecordHandlers, each site section has its corresponding record handler that handles communication with the database, and inherits from the RecordHandler superclass). Each RecordHandler's sublcass overrides one method, setValueArray(), which sets its values, which are the names of the rows in the table it will be dealing with.

    When the SiteManager recieves a request, it uses a factory (either DynamicContentFactory or RecordHandlerFactory) to determine which sections class to instantiate, bases on a given id parameter.

    If anything else isnt clear, just ask, otherwise, fire away, as long as its constructive criticism.
    From here on, it's instinctual...even straight roads meander.

  2. #2
    SitePoint Enthusiast
    Join Date
    Jun 2003
    Location
    Ljubljana, Slovenia
    Posts
    83
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I use the same Idea .. and it work perfectly .. the only think I did different is naming them differently .. hehe .. I use MODULES (for handling logic) and PLUGINS (to handle outputs) and I added COLLECTIONS (for data ) .. I thought that sometimes it happens that MODULES use different types of data .. for example three different types of content .. so thats why I created collections .. module assembles them and plugins formats the output ...

    and your Site Manager class I turned into a little framework I use to handle everything i need .. but btw .. I guess its working for both of us !

    bye

    Armando

  3. #3
    SitePoint Addict
    Join Date
    Mar 2002
    Location
    Los Angeles
    Posts
    325
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    armondo, how has your setup been as far as reusability? Have you been able to use this framework on multiple projects? Have you run acoss any major downsides?
    From here on, it's instinctual...even straight roads meander.

  4. #4
    SitePoint Enthusiast
    Join Date
    Jun 2003
    Location
    Ljubljana, Slovenia
    Posts
    83
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    not really . I'm only upgrading it .. from project to project .. usualy projects have different demands but the idea stands .. down side is maybe only that design is used in a classes not in a XSLT stylesheet .. since I write it really fast .. and these moduels can be actually copy pasted from one project to another .. and it will still work with minor modifications .. I dont say its the best .. I wont even say its good .. but it solves the case without big overhead of clases and stuff

    bye

    armando

  5. #5
    SitePoint Enthusiast
    Join Date
    Sep 2004
    Location
    ohio
    Posts
    26
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    !!!!!!!

    WOW! I can't believe I stumbled across this post! I was going to post a question asking for names of programs to create that image you just created! How lucky! So what did you make that in? I read in the title that it's UML. After some googling that looked like a viable option, but somewhat hard to learn. But now after seeing it applied to PHP, I know that's what I need. So what UML program did you make that in? Also, do you have recomendations of other UML programs do perform the same task other than what you used?

    Thanks!

  6. #6
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Roadie
    I want everybody to criticise the crap out of my class diagram for this app I'm building.
    Game on...

    Quote Originally Posted by Roadie
    Here's the link to my class diagram:
    Ok, I think I know where you are going with this. I have a few suggestions.

    Firstly, let's try to ditch some of the accessors. Create a Writer interface with a few simple methods, say writeRow(), writeHeaderRow(), writeText() say. The writer just executes print statements in the simplest case. Your output methods should accept the writer in the call. Take the getAllFromTable() method which probably causes you all sorts of data munging hassle...
    PHP Code:
    $outputter = new NewsOutputter();
    $outputter->writeNewsSummary(new HtmlWriter()); 
    The outpuuter can do all of the looping, etc, to dump the table: startTable(), writeRowHeader(), writeRow(), ..., stopTable(). The benefit? How do you know it's a table? It could be a list? The outputter decides, but the writer handles the formatting. You could have a textWriter() for e-mail news, or a PdfWriter().

    In addition your code will get a lot more cleaner and alot more flexible regarding the data structure. You are encapsulating not just the data, but the format of the data as well with this trick.

    Next problem is naming. What you usually call an Outputter is more often called a View. What you call a RecordHandler is more usually called a RecordSet. When you are instatiating RecordSet objects and their wrappers, the factories are more usually called Finders or Repositories. So ViewFactory makes sense (you don't know it's DB data), but RecordHandlerFactory could just be DocumentRepository or DocumentFinder or even Librarian. These are just naming conventions, but it allows people to look at your diagram and immediately understand the design.

    The final trick is to decouple the views and the SiteManger (ghastly name) with a dash of dependency injection. If you have more smaller views, say NewsSummaryView, rather than cutting the views into artificial structural blocks, you can standardise their constructors into just accepting the record sets they need.

    Say we define the interface like so...
    PHP Code:
    class NewsSummaryView {
        function 
    __construct(NewsSummaryRecordSet $records) {
            ...
        }

        function 
    showFullListing($writer) {
            ...
        } 
    Note that type hint. The ViewFactory can now do something clever. It can use reflection to read those type hints and then ask the LIbrarian for an object of that type. Say the data in the record set has not been fetched yet. The constructor of NewsSummaryView can then set criterai on the record set and then call fetch() on it.

    The point? You don't need SiteManager. This is a god class anyway and knows way to much about constructor parameters. This is too constraining and unnecessary for a dynamic language like PHP. beware on classes called Manager...or Controller .

    I would call Viewfactory something like ViewPopulator now, because it has a much more specific job to do.

    Note that I have also subtly moved more resposibility into the RecordSet classes. This is because tabular applications are likely to have all sorts of special queries that are probably quite tied to the data they are representing. An external finder that does more than instantiate classes will have to build queries that can be passed in. A query interface is a large and complicated thing and probably data specific. If we can keep that in the RecordSets then your top level design is less fussy. Your milleage may vary on this one though.

    Quote Originally Posted by Roadie
    If anything else isnt clear, just ask, otherwise, fire away, as long as its constructive criticism.
    Well, you can criticise my ideas back .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  7. #7
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    beware on classes called Manager...or Controller
    Or just about anything that ends with an er I'd think? There is a reference somewhere (lost the bookmark) that gives a number of reasons as to why not to use a name which ends with er, but Marcus is right (again )

    +2 Marcus

  8. #8
    SitePoint Addict
    Join Date
    Mar 2002
    Location
    Los Angeles
    Posts
    325
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    ozone - the app is called gModeler, it a free online tool. Check it out, i think its great.

    Marcus - great post! thanks, that was exactly the kind of stuff I was looking for. Some questions:

    I'm just a litte bit unclear about the how the View(what I called "outputter") works with the Writer interface. Would it be too much to ask for a more flushed out explanation of that part, so I can get a better idea? I think what are saying is that the View is still composed with a Recordset object (by the way, i used the term "RecordHandler" becasue I think of a RecordSet as a result from a query...the RecordHandler handled different queries on a db), and then hands that Recordset's data off to the Writer for formatting. So, there would be a different Writer for each section then?:
    PHP Code:
    $outputter->writeNewsSummary(new NewsHtmlWriter()); 
    Note that type hint. The ViewFactory can now do something clever. It can use reflection to read those type hints and then ask the LIbrarian for an object of that type.
    Hmm...well, I am using php4, and I don't think that has type hinting. But, since I am instansiating my classes based on a id that I passing to the factory(like "News"), couldn't I just have the factory get the RecordSet first, based on that id, and then pass that that RecordSet into the new View?

    Take the getAllFromTable() method which probably causes you all sorts of data munging hassle...
    no, not really, because each section of the site had its own table...to each outputter I created just asked it's record handler to return all the rows from the associated table. Each outputter then knew what to do with the records returned.
    From here on, it's instinctual...even straight roads meander.

  9. #9
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Roadie
    Would it be too much to ask for a more flushed out explanation of that part, so I can get a better idea?
    It really is as simple as I have painted it. Basically think of the writer as accepting generic paint operations. You have a HtmlWriter say (just one, not one for every view) taht can handle the typical atoms of presentation, such as text elements...
    PHP Code:
    class HtmlWriter {
        ...
        function 
    writeText($text) {
            print 
    htmlentities($text);
        }

        function 
    startTable() {
            print 
    '<table>';
        }
        ...

    Just accumulate them as you need them. this follows a 'tell don't ask' policy. The View just tells the writer what element to paint. The top level app. just tells the view to paint itself. No need to keep unpicking accessors and if the format changes, say a single table becomes two tables, this does not affect anything other than the View.

    Quote Originally Posted by Roadie
    I think what are saying is that the View is still composed with a Recordset object (by the way, i used the term "RecordHandler" becasue I think of a RecordSet as a result from a query...the RecordHandler handled different queries on a db), and then hands that Recordset's data off to the Writer for formatting.
    Could the RecordSet just do all of this itself? By having a handler you are exposing complications to the upper levels. Push detail down, push decisions up is a good motto when you want to split a class.

    Quote Originally Posted by Roadie
    So, there would be a different Writer for each section then?:
    No. Which writer to pass in (probably into the constructor if more than one View method is called) is the decision of the application. The View should not know it is writing to a web page or an e-mail or whatever. Also if you have four formats (HTML, PDF, RTF, RSS) and four views (NewsSummaryView, NewsArticleView, ReportView, StopPressView) you will end up with 16 writer classes. This is a class explosion and definitely something you don't want.

    Quote Originally Posted by Roadie
    Hmm...well, I am using php4, and I don't think that has type hinting.
    You are correct. In that case pass the Librarian into the newly created view, using say a load method...
    PHP Code:
    class ArticleView {
        ...
        function 
    load($librarian) {
            
    $query = new Query('articles');
            
    $query->setField('id'$this->_article_id);
            
    $this->_article_data = &$librarian->find($query);
        }

    Quote Originally Posted by Roadie
    But, since I am instansiating my classes based on a id that I passing to the factory(like "News"), couldn't I just have the factory get the RecordSet first, based on that id, and then pass that that RecordSet into the new View?
    The your manager class has to have this capability upgraded for every new view and record set. There is nothing to stop it kicking off this negotiation, but it's best to let the view decide what objects to load and let the higher levels supply the resources to do so.

    Quote Originally Posted by Roadie
    Each outputter then knew what to do with the records returned.
    Who handles unpacking the data and wrapping it in HTML tags? Is that part not on your diagram?

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  10. #10
    SitePoint Addict
    Join Date
    Mar 2002
    Location
    Los Angeles
    Posts
    325
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    ok, i think it's best if i answer your post in reverse order.

    Who handles unpacking the data and wrapping it in HTML tags? Is that part not on your diagram?
    that is taken care of in the outputter. Basically, flow would be like this:

    -the Manager would take in id variable (this id is set in the nav on the html page)

    -the Manager then calls the either the Outputter factory, or the RecordHandlerFactory, and instansiates the correct object based on the id that was passed in.

    Here is how the SiteManager looks. i took out some of hte methods, but the other ones look very similar to these:


    PHP Code:
    <?php


    class SiteManager{
        
        function 
    SiteManager(){}
        
        function 
    doConnect($host$user$pass$db){
            
    $conn mysql_pconnect ($host$user$pass);
        
            if (!
    $conn){
                echo 
    'Error connecting to db'.'<br>';
            }
            
            
    //select the right database to use
            
    $db mysql_select_db($db);
            
            if (!
    $db){
                echo 
    'Error selecting database'.'<br>';
            }
        }
        
        
        function 
    getDynamicContent($cat$sortBy$sortDirection$limit){
            require_once(
    "outputters/DynamicContentFactory.php");
            
    $outputter DynamicContentFactory::getDynamicOutputter($cat);
            
    $content $outputter->getContent($sortBy$sortDirection$limit);
            return 
    $content;
        }
        
        
        
        function 
    insertNewRecord($section$withFile){
            
    //the $withFile parameter is letting us know there is a file to upload as well
            
    require_once("record_handlers/RecordHandlerFactory.php");
            
    $recordHandler RecordHandlerFactory::getRecordHandler($section$withFile) or die('Error Getting Record Handler');
            
    $res $recordHandler->insertRecord();
            
            if (
    $res){
                
    $message 'Record inserted successfully.';
            }else{
                
    $message 'Error inserting record.';
            }
            
    $this->onRecordResult($message);
        }

    }

    Now, the outputter gets created in the getDynamicContent() method autmatically gets composed with its corresponding RecordHandler...so for the News_Ouputter object, it gets composed with a News_RecordHandler on creation. I have a different *_Outputter and *_RecordHandler for each section.

    So, when I call $outputter->getContent(), the $outputter calls a method on it's RecordHandler. The record handler retireves the records from it's assigned table(each table in the database also has the corresponding id for each section...so there is a site_news table, site_about table, etc.) and returns the results to the outputter. The outputter then formats the data and outputs that formatted data.. Here is how it looks in the News_Outputter:
    PHP Code:
    function getContent($sortBy$sortDirection$limit){
            
            
    $rh $this->recHandler;
            
    $res $rh->getAllFromTable($sortBy$sortDirection$limit);
            
            
    $this->outString "";
            
            while(
    $row mysql_fetch_array($res) ){
            
                
    $newsDate $row['date'];
                
    $blurb $row['blurb'];
                
                
    $this->outString .= '<p><div class="date">'$newsDate '</div>';
                
    $this->outString .= $blurb;
                
    $this->outString .= "<br /><br />";
            }
            return 
    $this->outString;
        } 

    The your manager class has to have this capability upgraded for every new view and record set.
    Hmm..how so? All the names of my classes correspond to the id being passed in...it just takes the id and asks the factory for the right object. Here's what the entire factory looks like:

    PHP Code:
    <?php 

    require_once("DynamicContentOutputter.php");

    class 
    DynamicContentFactory{
        
        function 
    DynamicContentFactory(){}
        
        function 
    getDynamicOutputter($id){
        
            
    $id strtoupper($id);
            
            
    $classFile "{$id}_Outputter.php";
            
    $class $id.'_Outputter';
            
            require_once( 
    $classFile );
            return new 
    $class($withImage);
                
        }

    }

    The View should not know it is writing to a web page or an e-mail or whatever.
    ok, i thought i was following along but now i'm really confused. How could the View NOT know what it's writing to? I would think it would need to call very different methods for printing to an html page than printing to an rss feed?
    From here on, it's instinctual...even straight roads meander.

  11. #11
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I would think it would need to call very different methods for printing to an html page than printing to an rss feed?
    The implementation would be different in each format, but the interface remains the same all the same. The client script that requires the use of the underlying classes would expect to use the same API for HTML, as it would for RSS.

    As Marcus has just said, the client shouldn't know about the output method,...

    The View should not know it is writing to a web page or an e-mail or whatever.

  12. #12
    SitePoint Wizard REMIYA's Avatar
    Join Date
    May 2005
    Posts
    1,351
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Looks like everything is fine within the UML

  13. #13
    Massimiliano Bruno Giordano sid egg's Avatar
    Join Date
    Aug 2004
    Location
    Canada
    Posts
    1,280
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    For the HTML writer class, were you thinking something like

    PHP Code:
    <?
    class HTMLWriter {
        var 
    $mode[];    
        var 
    $tags[];
        
        function 
    HTMLWriter($type="XHTML") {
            
    $mode['pageType'] = $type;
            
    $mode['pageStatus'] = "NOPAGE";
        }
        function 
    startPage() {
            if(
    $mode['pageType'] == "XHTML") {
                print 
    '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">';
            } else {
                print 
    '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">';
            }
            print 
    "<html>";
            
    $mode['pageStatus'] = "PAGE";
        }
        
        function 
    br() {
            if(
    $mode['pageType'] == "XHTML") {
                print 
    "<br />";
            } else {
                print 
    "<br>";
            }
        }
        
        function 
    openTag($tag$parameters) {
            
    $tags[] = $tag;
            print 
    "<$tag $parameters>";
        }
        
        function 
    closeTag($tag) {
            
    //hmm, how to clear the array... whatever, this is just an example anyways, I
            //think I'll make each tag an instance of class HTMLTag later... maybe
            //remove $tag from $tags[]
            
    print "</$tag>";
        }
        
        function 
    endPage($closeAll) {
            if(
    $mode['pageStatus'] != "PAGE") {
                
    trigger_error("Error [HTMLWriter::endPage]: No page to close.");
            }
            if(
    $closeAll) {
                for(
    $i=0;$i<count($tags);$i++) {
                    print 
    "</" $tags[$i] . ">";
                }
            }
            print 
    "</html>";
        }
        function 
    writeText($text) {
            if(
    $mode['pageStatus'] != "PAGE") { 
                
    trigger_error("Error [HTMLWriter::writeText]: Text should not be written outside of a page. HTMLWriter::startPage() should be called first!");
            }
            print 
    htmlentities($text);
        }
        
        function 
    startTable($padding$spacing$title="", ) {
            
    $tags[] = "table";
            print 
    "<table title='$title' cellpadding='$padding' cellspacing='$spacing'>";
            
        }
    }
    ?>
    Edit, oooooh! Found a Javadoc: http://java.sun.com/j2se/1.3/docs/ap...TMLWriter.html
    GamesLib.com - the slickest, most complete and
    easily navigatible flash games site on the web.

  14. #14
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Roadie
    that is taken care of in the outputter.
    But something has to loop through the result set, right? If the result set is a DB cursor, then that means goinbg through an iterator interface to build each row, right? Wher is thet loop? I agree it should be in the View (Outputter), but I am trying to get teh actual HTML into a writer so that the two don't get munged together.

    Quote Originally Posted by Roadie
    Here is how the SiteManager looks. i took out some of hte methods, but the other ones look very similar to these:
    It's got a lot of responsibilities. I don't have a direct criticism here, but it makes me uneasy. For example the site manager knows that a relational DB is being used and which one. Surely you don't want to be rewriting this complex class on each project. You just want to write views I think.

    Quote Originally Posted by Roadie
    Now, the outputter gets created in the getDynamicContent() method autmatically gets composed with its corresponding RecordHandler...so for the News_Ouputter object, it gets composed with a News_RecordHandler on creation.
    What if a view has multiple result sets to display?

    As an aside, how are the articles edited? Do you have a NewsEditView or some such?

    Quote Originally Posted by Roadie
    Hmm..how so? All the names of my classes correspond to the id being passed in...it just takes the id and asks the factory for the right object.
    It actually looks like you have a name mangling solution, which is similar to DI, but without type handling.It does strike me taht you are doing the same step twice though. Firstly to get the view, and then to get the data. Why not just have the View as for it's data from some kind of repository. You already have all of the information in the view, so don't need any kind of autowiring at this point.

    Or have I misunderstood?

    Quote Originally Posted by Roadie
    ok, i thought i was following along but now i'm really confused. How could the View NOT know what it's writing to? I would think it would need to call very different methods for printing to an html page than printing to an rss feed?
    Not necessarily, if you describe the output in very general terms, such as tables and rows. For example a web script...
    PHP Code:
    <?php
        $views 
    = new ViewBuilder();
        
    $view $views->createView('LatestArticles', new HtmlWriter());
    ?><html>
        <?php $view->showTitles(); ?>
    </html>
    ...or when sending a mail...
    PHP Code:
    class Newsletter {
        function 
    mailHeadlines($contacts) {
            
    $views = new ViewBuilder();
            
    $view $views->createView('LatestArticles', new TextWriter());
            foreach (
    $contacts as $contact) {
                
    $this->mail($contact->getEmail"Hi " $contact->getName() . "\n" .
                        
    "Here are todays headlines...\n" $view->showTitles());
            }
        }

    The View doesn't care whether it's show() method is writing to HTML or Text. It could just be this...
    PHP Code:
    class LatestArticles {
        ...
        function 
    showTitles() {
            
    $this->_writer->startList();
            
    $iterator $this->_results->getCursor();
            while (
    $row $iterator->next()) {
                
    $this->writer->writeItem($row['title']);
            }
            
    $this->_writer->endlist();
        }

    The output format is chosen by the top level app. (as it should be), but the details of this process are in the View and the Writer choices. That's what I mean about promoting decisions (up to the top level script in this case), but pushing down details (was it a list or a table? who cares).

    It's up to you of course.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  15. #15
    Knowledge is key 2 progression Tryst's Avatar
    Join Date
    Sep 2003
    Location
    Wales
    Posts
    1,181
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Are there any books or online articles which discuss and explain View and Writer Classes?

    Tryst


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
  •