Grasping OOP - Take a look

[geeknik]

Hello folks,

I've been programming PHP for a pretty long while. Most of my code until recently has been procedural. I am trying to get a grasp on OOP so I decided to write a little directory listing class. I know this could be done tons easier just uisng the psuedo class PHP already provides but I am still trying to get a feel for how to use OOP in my applications. Take a look at my code and see what I could do to make this better.

<?php
    //    ===================================================================
    //    Class - Directory Lister
    //    One of my first attempts at grasping OOP PHP.
    //    October 30, 2004
    //    ===================================================================
    
    class DirectoryReader {
        //    Member Property Declarations
        //=================================================================
        var $directory;    //    This is the directory we wish to list.
        var $dirRsrc;    //    Directory Resource.
        var    $entry;        //    This is the current entry in the directory list.
        var $output;    //    This will hold the html to return to the page for printing.
        
        //    Constructor: Takes a directory as the argument.
        function DirectoryReader($dir) {
            $this->directory = $dir;
            $this->dirRsrc = opendir($this->directory);
            $this->output = '';
            $this->createList();
        }//    End Constructor.
        
        //    This member function will read the directory.
        function read() {
            readdir($this->dirRsrc);
        }
        
        //    This function will create the directory list.
        function createList() {
            //    Start the list HTML
            $this->output .= '<h1>File Listing</h1><ul>';
            //    Loop through the directory.
            while($this->entry = readdir($this->dirRsrc)){
                if(is_file($this->directory . $this->entry)){
                    $this->output .= '<li><a href="' . $this->directory . $this->entry . '">' . $this->entry . '</a></li>';
                }
            }
            //    End the list HTML
            $this->output .= '</ul>';
        }
        
        //    This function will return the output.
        function getList() {
            return $this->output;
        }
        
        
    }//    End DirectoryList Class
        
?>

[Willem69]

You're creating HTML code during directory listing, it's better to split those.
One method to get the contents, and one method to parse it into HTML code.

That way if you would ever change the listing methode (eg using 'glob' instead of opendir & readdir, you wouldn't have to change the code which creates the HTML code.

Further more if you split those, you could install another small xml or html-template if you liked.

And your starting to read as soon as you create an instance.
Don't start opening en reading of right away, cause you might want to 'configure' the class somehow (like only show image files).

At this point you're executing almost all class-methods when you create an instance.
That's quite procedural too

Here's an idea:

$dirlist = new DirectoryReader;
$dirlist->setValidTypes('jpg','gif','png');
$dirlist->readDir('/path/to/directory') or die($dirlist->getError());
$dirlist->setTemplate('<file><name>%1\$s</name><path>%2\$s</path></file>');

// start output
header('Content-Type: application/xml; charset=iso-8859-1');
print '<?xml version="1.0" encoding="iso-8859-1"?><files>';
$dirlist->printList();
print '</files>';

Most important is that your code remain flexible, reusable and readable

[Version0-00e]

The idea I get from the need to pull data that is recursive is to apply the Iterator pattern.

Have a look at the Iterator pattern on some Java documentation, and/or look at the pattern on Harry's website, http://www.phppatterns.com for more on this pattern, and if I remember, theres a few examples as well

[geeknik]

Hey thanks a lot guys!

I've changed my code a little after taking some advice from the people in the parent forum. I never completely caught on to recursion in school, I will have to take a look at phppatterns to learn some more. I am also interested in the setTemplate() method described in Willem69's reply. Maybe you could help me understand how that would be implemented? Here is my updated code. Let me know how I can apply some good changes, I know there are plenty !

//    =================================================================== 
    //    Class - Directory Lister 
    //    One of my first attempts at grasping OOP PHP. 
    //    October 30, 2004 
    //    =================================================================== 
    //    Sample Usage: 
    //    $dir = new DirectoryReader(); 
    //    $fileList = $dir->getList('/some/directory/'); 
    //        echo '<li>'; 
    //        foreach($fileList as $file){ 
    //            echo '<li>$file</li>'; 
    //        } 
    //        echo '</ul>'; 
    // 
    // 
     
    class DirectoryReader { 
        //    Member Property Declarations 
        //================================================================= 
        var $directory;    //    This is the directory we wish to list. 
        var $dirRsrc;    //    Directory Resource. 
        var    $entry;        //    This is the current entry in the directory list. 
        var $output;    //    This will hold the html to return to the page for printing. 
         
        //    Constructor: Takes a directory as the argument. 
        function DirectoryReader() { 
        }//    End Constructor. 
         
        //    This member function will read the directory. 
        function read($dirRsrc) { 
            return readdir($dirRsrc); 
        } 
         
        //    This function will create the directory list. 
        function createList($dir) { 
            $this->directory = $dir; 
            $this->dirRsrc = $this->read($this->directory); 
            $this->output = array(); 
            //    To keep markup code out of this class 
            //    we will set it up to return an array. 
            //    $output[] will now be an array. 
            while($this->entry = $this->read($this->dirRsrc)){ 
                if(is_file($this->directory . $this->entry)){ 
                    $this->output[] = $this->directory . $this->entry; 
                } 
            } 
        } 
         
        //    This function will return the output. 
        function getList($dir) { 
            $this->createList($dir); 
            return $this->output; 
        } 
         
    }//    End DirectoryList Class 

[soapsud]

i'd definitely second the iterator recommendations. anything that involves looping over an abstract set of data profits well from an iterator. you then have a simple interface to get at any sort of data easily. essentially you'd be going for code which looks like this:

class DirectoryIterator {

  (void) function DirectoryIterator($newDir);

  (boolean) function next();

  (string) function get();

  (void) function reset();

} 

so the usage would be rather simple in the end:

$it =& new DirectoryIterator("/path/to/dir");
while ($it->next() ) {
  print $it->get();  // current file name in the data set
}