Working through some OOP with Peter Lavin
clintonia
I'm at the end of Chapter 5 in Peter Lavin's OO PHP book. I'm left
with some seemingly huge questions, especially after downloading his
source code (which I hated to have to do, but mine wasn't working).
First off, here's my class for going through a directory to check for
images.
<?php
class DirectoryItems
{
private $filearray = array();
//Constructor
public function __construct($directory, $replacechar = "_")
{
$this->directory = $directory;
$this->replacechar = $replacechar;
$d = "";
if(is_dir($directory))
{
$d = opendir($directory) or die("Could not open directory.");
while(false !== ($f=readdir($d)))
{
if(is_file("$directory/$f"))
{
$title = $this->createTitle($f);
$this->filearray[$f] = $title;
}
}
closedir($d);
}else
{
//error
die("Must pass in a directory.");
}
}//End Constructor
private function createTitle($title)
{
//strip extension
$title = substr($title, 0, strrpos($title, "."));
//replace word separator
$title = str_replace($this->replacechar, " ", $title);
return $title;
}
//////////////////////////////////////////////////
function indexOrder()
{
sort($this->filearray);
}
//////////////////////////////////////////////////
function naturalCaseInsensitiveOrder()
{
natcasesort($this->filearray);
}
//////////////////////////////////////////////////
function getCount()
{
return count($this->filearray);
}
//////////////////////////////////////////////////
public function filter($extension)
{
$extension = strtolower($extension);
foreach ($this->filearray as $key => $value)
{
$ext = substr($key, (strpos($key, ".")+1));
$ext = strtolower($ext);
if($ext != $extension)
{
unset($this->filearray[$key]);
}
}
}
//////////////////////////////////////////////////
public function checkAllSpecificType($extension)
{
$extension = strtolower($extension);
$bln = true;
$ext = "";
foreach($this->filearray as $key => $value)
{
$ext = substr($key, (strpos($key, ".") +1));
$ext = strtolower($ext);
if($extension != $ext)
{
$bln = false;
break;
}
}
return $bln;
}
//////////////////////////////////////////////////
public function getFileArray()
{
return $this->filearray;
}
//////////////////////////////////////////////////
public function removeFilter()
{
unset($this->filearray);
$d="";
$d = opendir($this->directory) or die("Couldn't open directory.");
while(false !== ($f = readdir($d)))
{
if(is_file("$this->directory/$f"))
{
$title = $this->createTitle($f);
$this->filearray[$f] = $title;
}
}
closedir($d);
}
//////////////////////////////////////////////////
public function checkAllImages()
{
$bln = true;
$extension = "";
$types = array("jpg", "jpeg", "gif", "png");
foreach($this->filearray as $key => $value)
{
$extension = substr($value, (strpos($value, ".") +1));
$extension = strtolower($extension);
if(!in_array($extension, $types))
{
$bln = false;
break;
}
}
return $bln;
}
//////////////////////////////////////////////////
public function imagesOnly()
{
$extension = "";
$types = array("jpg", "jpeg", "gif", "png");
foreach($this->filearray as $key => $value)
{
$extension = substr($value, (strpos($value, ".") +1));
$extension = strtolower($extension);
if(!in_array($extension, $types))
{
unset($this->filearray[$key]);
}
}
}
}
?>
And here's my page that calls the class.
<?php
require 'DirectoryItems.php';
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://
www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=iso-8859-1" />
<title>Files on my system</title>
</head>
<body>
<?php
$di = new DirectoryItems("images");
if(!$di->checkAllImages()){
$di->imagesOnly();
}
$di->naturalCaseInsensitiveOrder();
//get array
echo "<div style = \"text-align: center;\">";
foreach($di->filearray as $key => $value)
{
echo "<img src=\"images/$value\" /><br />\n\r";
}
echo "</div><br />";
?>
</body>
</html>
I get this error:
Fatal error: Cannot access private property DirectoryItems::$filearray
in C:\wamp\www\clintstuff\items.php on line 20
Now, there are few differences in what he has in the book (which is
what I've done) and what he has online for his source code, namely a
destructor and declaring a few more variables as private. Is the
destructor mandatory?
//////////////////////////////////////////////////////////////////
//destructor
//////////////////////////////////////////////////////////////////
public function __destruct(){
unset($this->filearray);
}
He's also added:
private $replacechar;
private $directory;
Why? (and shouldn't that be mentioned in the book?)
Also, if I could get some pointers on my class and why it's not
working that'd be great. I'm avoiding just copying code so that it
works.
Thanks!
Michael Girouard
As for improvements, well, there are a few problems that I have with that class. First of all being that it uses lots of procedural code in a class just to accomplish something that could be done more simply without a class. I recommend looking into PHP's dir() ( http://php.net/dir) function and seeing just how easy it is to iterate over a directory of items.
Secondly, the object is poorly designed. There are functions there with complex filtering logic to determine images vs other file types and that's simply not necessary. If this class were to be broken up into multiple classes, the code would be simplified and filtering would be a trivial task.
When you see enormous classes like this, generally they can be described as "God Objects" or in other words, objects that try to do everything. In a programmer's world, this is a nightmare in terms of scalability.
If I were to design this code, I would have each file in the $filearray property be an instance of a class which describes it's type specifically. This way you could use PHP's instanceof operator and filter appropriately. This unfortunately can't work with the current class because the files are stored as simple strings. Thus, a developer must result to some pretty intense string parsing just to do simple tasks... yuck!
As an added benefit, each class that describes a particular file could have file-specific functionality. For example, HTML and XML files could implement a proprietary asXML() method that returns a SimpleXML or DOMDocument instance. Images could also benefit as they could implement an interface that provides methods to resize, crop, mask, and change to another format.
This sounds like a really fun library to work on. I wonder if something similar has been made yet... perhaps a PEAR package already exists?
Mike Geee!
Professional Superhero
mgir...@gmail.com
386-503-3505
Me: www.lovemikeg.com
Us: www.cfphp.org
clintonia
understanding, yay!
since:
$this->filearray[$f] = $title;
in the Constructor, all:
$extension = substr($value, (strpos($value, ".") +1));
needs to be changed to:
$extension = substr($key,(strpos($key, ".")+1));
Also,
foreach($di->filearray as $key => $value)
{
echo "<img src=\"images/$value\" /><br />\n\r";
}
needed to be changed to:
$filearray = $di->getFileArray();
foreach($filearray as $key => $value)
{
echo "<img src=\"images/$key \" /><br />\n\r";
}
as $di->filearray was trying to directly access a private function
(not allowed), whereas $di->getFileArray() is a public function
accessing an internal private function (which is allowed).
Please correct me or add to this if I missed something in explaining.
Whew! on to Chapter 6.
Regards,
Clint
clintonia
Looks like we were typing at the same time! =)
Thanks for the tips, I'll be going back through and trying to improve
this.
Mike G.
I wrote a really simple set of classes which describe the library I
was talking about. Mind you, these are not functional but at least
provide a better example of what I was talking about in my previous
post in this thread.
http://cfphp.googlegroups.com/web/Files.php
I encourage any questions/comments/edits/etc.
Clint:
If the above file were actually fleshed out completely, the improved
filter code would be:
public function filterList($type) {
/* convert a mime to a class name */
$instanceName = 'FileType_'.str_replace('/', '_', $type);
foreach($this->files as $key => $file) {
if($file instanceof $instanceName) {
unset($this->files[$key];
}
}
}
Which is totally scalable and requires not extension checking. What's
really cool about it is that it uses PEAR-style namespacing which
lends itself well to the Apache-style mime types. So an image/gif mime
type can quickly become Image_GIF (the class name suffix for GIF
images) by replacing the slash with an underscore.
Cheers,
Mike G.