PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Mon Sep 28, 2020 2:00 pm

All times are UTC - 5 hours




Post new topic Reply to topic  [ 9 posts ] 
Author Message
PostPosted: Sat Jul 29, 2006 1:19 pm 
Offline
DevNet Resident
User avatar

Joined: Tue Jan 20, 2004 5:58 pm
Posts: 1537
Location: Minnesota
Hey all I'm trying to learn PHPDocumentor today. I've written some. Please critque. I'm wondering did I use the tags right? Are there tags I could use better and are my comments useful?

Thanks!

Syntax: [ Download ] [ Hide ]

<?php

/**

* Gallery Controller for GOOP Gallery ver 2.0

*

* This is the contolling class for GOOP Gallery. It process requests and

* instantiates the "view" or module that is requested. Communication

* between the two main classes dynamic_image and dynamic_gallery are done

* through the controller.

*

* Copyright (C) 2004 - 2006 by Eugene Ritter

*

* This program is free software; you can redistribute it and/or modify

* it under the terms of the GNU General Public License as published by

* the Free Software Foundation; either version 2 of the License, or

* (at your option) any later version.

*

* This program is distributed in the hope that it will be useful,

* but WITHOUT ANY WARRANTY; without even the implied warranty of

* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

* GNU General Public License for more details.

*

* You should have received a copy of the GNU General Public License

* along with this program; if not, write to the Free Software

* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

*

* _________________________________________________________________________

*

* GOOP Gallery is available at http://www.webgeneius.com

* Comments & suggestions: http://www.webgeneius.com/contact.php

* With special thanks to devnetwork.net for all the support they've

* provided in my efforts to master PHP.

*

* @author Eugene Ritter

* @version 2.0.1

* @package goopgallery

*/




class gallery_controller {

/**

* The configuration variable for this class.

* Loaded from gallery_config.php

* @access private

*/


    var $CFG;

/**

* The path to various gallery modules

* @access private

**/


    var $module_path;

/**

* Holds the name of the gallery requested

* @access private

*/


    var $gallery;

/**

* Holds the request string variables

* @access private

*/


    var $request;





/**

* Constructor sets up $con

* @access public

* @uses module_config.php

* @uses dynamic_files.php

* @uses gallery_config.php

*/


        function gallery_controller(){

                require_once('gallery_config.php');

                if(isset($CFG)){

                        $this->set_global($CFG,'CFG');

                }

                $this->CFG = &$CFG;

                $this->module_path = $CFG->absolute_path . "modules/";

                $this->get_request();

                $this->set_global($this->request, 'request');

                $this->CFG->module = (!empty($this->CFG->module) && (!strcasecmp($this->CFG->module, 'default') == 0) && $this->module_exists($this->CFG->module))? $this->CFG->module : 'table';

                require_once($this->module_path . $this->CFG->module . "/module_config.php");

                require_once($CFG->absolute_path.'lib/dynamic_files.php');

                $df = new dynamic_files();

                $this->set_global($df, 'df' );

        }

/**

* Adds objects and variables to $GLOBALS

* @access public

* @param object $object the object to be inserted into the $GLOBALS container

* @param string $handle optional name of class to be made globally accessible

*/
     

        function set_global( &$object, $handle = '' ){

                $class = (empty($handle))? get_class($object): $handle;

                $GLOBALS[$class] = &$object;

        }

/**

* A "getter" for retrieving objects

* @param string $handle The name of the class to retrieve from the $GLOBALS array

*/


        function get_global($handle){

                return $GLOBALS[$handle];

        }

/**

* Calculate all of the request variables and stores them in the request property.

* Sets them equal to empty/null even if they're not used.

* Request contains the following default properties: id, gallery, image, large, page

* The module requested is stored in the $CFG.

* @access public

* @param array $additional The names of additional request names to be added to the $request object

*/


    function get_request($additonal=array()){

        $this->request->id = (isset($_REQUEST['id']))? intval($_REQUEST['id']) : '';

        // gallery

        $this->request->gallery = (isset($_REQUEST['gallery']))? urldecode($_REQUEST['gallery']): '';

        // image

        $this->request->image = (isset($_REQUEST['image']))? urldecode($_REQUEST['image']): '';

        // large

        $this->request->large = (isset($_REQUEST['large']))? $_REQUEST['large']: '';

        // page

        $this->request->page = (isset($_GET['page']))? intval($_REQUEST['page']): '';

        // module

        $this->request_module();

        //If you don't like the preset query values make you're own!

        if(!empty($additional)){

                foreach ($additional as $name => $value ){

                        $this->request->$name = urldecode($value);     

                }              

        }

    }

/**

* This function calculates what module has been requested.

* $_POST methods override $_GET methods.

* $_COOKIES are also set and used to determine the requested module

* @access private

*/


    function request_module(){

        if (isset($_GET['module'] || $_POST['module'] )) {

                $module = ( $_POST['module'] ) ? $_POST['module'] : $_GET['module'];

                if ($this->module_exists($module)) {

                        $this->set_module_cookie($module);

                        $this->CFG->module = $module;

                }

        } else {

                if (isset($_COOKIE['module'])) {

                        if ($this->module_exists($_COOKIE['module'])) {

                                $this->CFG->module = $_COOKIE['module'];

                        }

                }

        }

    }

/**

* Sets a cookie named module that holds the name of the module requested.

* @access private

* @param string $module contains the name of the module requested.

*/


    function set_module_cookie($module){

        setcookie('module', $module, time() + 60 * 60 * 24 * 30, '/', $this->CFG->domain);

    }

/**

* See if the module requested is the real thing or not.

* @access private

* @param string $module The name of the module to be tested.

* @return boolean

*/


    function module_exists($module){

        return is_dir($this->module_path . $module);

    }

/**

* This is an interface class. All modules are required to have a gallery_maker() method.

* Selects the modules, includes their code and instantiates them.

* The request logic for the default module 'table' is stored here.

* Request logic for other modules are contained in a requried method called 'gallery maker'

* @access public

* @uses dynamic_gallery.php

*/


    function gallery_maker(){

        require_once('dynamic_gallery.php');

        // Default table module load the gallery and handle requests

        if ($this->CFG->module == 'default' || $this->CFG->module == 'table') {

            // Load up the default gallery class.

            $this->gallery = new dynamic_gallery();

            if (($this->request->page != null || $this->request->gallery != null) && $this->request->large == null) {

                // prepare first view

                if ($this->gallery->get_files()) {

                    $this->gallery->first_view();

                }

            } elseif ($this->request->large != null) {

                $this->gallery->second_view($this->request->image);



            } else {

                $this->gallery->default_view();  

            }

            $this->gallery->output_gallery();

        } else {

            include($this->module_path . $this->CFG->module . '/module_gallery.php');

            $this->gallery = new module_gallery($this->request, $this->CFG);

            // If its a module request logic is stored

            // inside the module class in a function called

            // gallery_maker

            $this->gallery->gallery_maker();

        }

    }

/**

* Get file size, demensions and other information about an image

* @access public

* @param string $path Path to the image to get information about

* @param string $file The file name to get information about

*/


    function get_image_info($path, $file){

        require_once('dynamic_image.php');

        if (!isset($dynamic_image)) {

            $dynamic_image = new dynamic_thumbs($this->CFG);

        }

        return $dynamic_image->get_image_info($path, $file);

    }

/**

* Instantiates dynamic_thumbs

* @access public

* @uses dynamic_image.php

* @param string  $dir The path to the image

* @param string  $file The name of the image

* @param boolean $large Create a large image true or false?

* @param boolean $save  Save the file into a directory?

*/


    function make_images($dir, $file, $large = false, $save = false){

        require_once('dynamic_image.php');

        if (!isset($dynamic_image)) {

            $dynamic_image = new dynamic_thumbs($this->CFG);

        }



        if ($large === false) {

            $dynamic_image->set_thumbs($dir, $file, $save);

        } else {

            $dynamic_image->get_image_info($dir, $file);



            $dynamic_image->create_image(true);

        }

    }

} //end class

?>



 


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 2:43 pm 
Offline
Forum Contributor
User avatar

Joined: Wed Aug 31, 2005 5:58 pm
Posts: 396
Location: Arkansas, USA
The comments are good.

What you should refactor some is the code itself. Many times, comments are just fragrance for a . Glancing over your class, I notice a couple of places where the code would be a lot cleaner if it was in its own class. It's much easier to have too few classes than too many, you know. :)

If you'd like a starting place, extracting the get_request method looks like a nice place to gain readability without too much work. Try porting that into a gallery_request class or similar.

Hope that helps :)

- Nathaniel


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 2:57 pm 
Offline
DevNet Resident
User avatar

Joined: Tue Jan 20, 2004 5:58 pm
Posts: 1537
Location: Minnesota


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 3:55 pm 
Offline
Forum Contributor
User avatar

Joined: Wed Aug 31, 2005 5:58 pm
Posts: 396
Location: Arkansas, USA


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 5:10 pm 
Offline
DevNet Master
User avatar

Joined: Mon Oct 25, 2004 9:29 pm
Posts: 3698
Location: New Jersey, US
I actually use Doxygen, but I think I can still comment on documentation style.

Personally, when I document classes, I keep the comments with the same indent as the code, as well as indent the asterisks:

Syntax: [ Download ] [ Hide ]
/**

 * Summary

 *

 * ...

 */


In terms of the documentation, I would advocate using the summary feature of PHPDocumentor: first line, describe the method as tersely as possible without repeating what can be determined from the function name, then go into detail. Furthermore, I wouldn't put the licensing information in the documentation: a developer trying to find out something about the API won't appreciate the GPL notice plastered over the page.

Make sure you compile your documentation. Otherwise, you have no idea whether or not it worked ;-)

Last I checked, PHPdoc style paragraphs need a double-space in order to be demarcated:

Syntax: [ Download ] [ Hide ]
/**

 * Summary

 *

 * Par 1

 * Not par 2

 */




/**

 * Summary

 *

 * Par 1

 *

 * Par 2

 */


I completely agree with Nathaniel: refactoring also would be a good idea.


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 5:44 pm 
Offline
DevNet Resident
User avatar

Joined: Tue Jan 20, 2004 5:58 pm
Posts: 1537
Location: Minnesota


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 5:50 pm 
Offline
DevNet Master
User avatar

Joined: Mon Oct 25, 2004 9:29 pm
Posts: 3698
Location: New Jersey, US


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 6:42 pm 
Offline
DevNet Resident
User avatar

Joined: Tue Jan 20, 2004 5:58 pm
Posts: 1537
Location: Minnesota
Post it up! Although code reviews are time consuming I'm finding it worth it. I think there is a lot to be learned from productive critique of code. Although you might find more response from posting in segments whether than multiple classes and files all at once.


Top
 Profile  
 
 Post subject:
PostPosted: Sat Jul 29, 2006 6:55 pm 
Offline
DevNet Master
User avatar

Joined: Mon Oct 25, 2004 9:29 pm
Posts: 3698
Location: New Jersey, US


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 9 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group