Page 1 of 1

PHP 5 Theory and Critique

Posted: Tue Mar 14, 2006 11:31 am
by neophyte
I've written this little class to glob some files in PHP5. The question is how well does this class follow PHP OO design principles. Is there something I could have done better? Should it have some exception catching some where. Would it be better with an interface or an abstraction? and so forth... Or is there something in here that's just plain nutty!

Code: Select all

class globFiles{
	protected $ext = array();
	protected $files = array();
	protected $path;
	public function __construct($ext = array()){
		$this->ext = $ext;
	}
	protected function globbers(){
		if(!empty($this->ext)){
			$extPattern = implode( ',*.', $this->ext);
			$pattern = '{*.';
			$pattern .= $extPattern.'}';
			$this->files = glob( $this->path.$pattern, GLOB_BRACE);
		}
	}
	public function setGlobPath($path){
		if (!strcmp($this->path{strlen($this->path)-1}, '/' )==0 ){
			$path .= '/';
		}
		if(is_dir($path)){
			$this->path = $path;
			return true;
		} else {
			return false;	
		}
	}
	public function getGlob(){
		$this->globbers();
		return $this->files;
	}
}

//Should this be apart of the globFiles? or should it be a child class?
class globFilesInfo extends globFiles{
//Secondary class to add information about the files (filectime, filesize)  to the files property.
}

Example of usage:

Code: Select all

//make an instance of the object.
$gf = new globFiles( array( 'gif', 'jpg', 'png') );

//set the path to the directory to be globbed
if ( $gf->setGlobPath('../devnetIcons') ){
	$files = $gf->getGlob();
//loop through the returned array
	foreach($files as $key => $val){
		echo "<img src=\"$val\" />";	
	}
}
Critique please.

Thanks! :wink:

Posted: Tue Mar 14, 2006 11:43 am
by John Cartwright
I would probably use another setter for the extention aswell, or instead just pass the filename in the constructor as well. Makes no sense to me having a setter for only half the variables.

Posted: Tue Mar 14, 2006 12:05 pm
by Chris Corbyn
Jcart wrote:I would probably use another setter for the extention aswell, or instead just pass the filename in the constructor as well. Makes no sense to me having a setter for only half the variables.
Since this is PHP5:

__set(), __get() :)

EDIT | Meh scrap that, it's pointless on such a small class.

Posted: Tue Mar 14, 2006 3:46 pm
by neophyte
Okay here is the class after some more additions.

I think I have set and get methods now for every property that makes sense to have one for.

I have two methods with die statements. Am I correct in assuming these would be better written with try and catch? And if so how? How's my naming conventions?

d11 how would I implement _call and _get in this class?

Code: Select all

class globFiles{
	protected $ext = array();
	private $files = array();
	private $path;
	private $pattern;
	private $filesWithInfo = array();
	private $imageExt = array('jpg',
							  'JPEG',
							  'bmp',
							  'BMP',
							  'png',
							  'PNG',
							  'GIF',
							  'gif',
							  'giff' 	);
	private $fileProperties = array (	'basename',
										'realpath',
										'dirname',
										'filesize',
										'filectime',
										'fileowner',
										'filegroup',
										'is_writeable',
										'is_readable',
										'is_executable');

	private function globbers(){
		if(!empty($this->pattern)) {
			$this->files = glob($this->path.$this->pattern);	
		}
		elseif(!empty($this->ext)){
			$extPattern = implode( ',*.', $this->ext);
			$pattern = '{*.';
			$pattern .= $extPattern.'}';
			$this->files = glob( $this->path.$pattern, GLOB_BRACE);
		}else{
			die('Pattern or ext list required.');
		}
		
	}
	private function getGlobFileExt($file){
		return substr($file, strrpos($file, '.')+1);
	}
	private function globInfo($file){
		//gets file specific information
		//check the extension and try to get image info if possible;
		foreach ($this->fileProperties as  $func){
			$this->filesWithInfo[$func] = $func($file);
		}	
	}	
	private function addGlobInfo(){
		//loop through files array add filectime() filesize() filemode() and image info if possible.
		foreach ($this->files as $key =>$val){
				$ext = $this->getGlobFileExt($val);
				if(in_array($ext, $this->imageExt)){
					//try to get image information	
					if($imgInfo = getimagesize($this->file)){
						//make the imageinfo array tied to useful keys
						//add it to $fileWithInfo	
						
						foreach ($imgInfo as $k => $info){
							switch($k){
								case 0:
									$this->fileWithInfo['image_width']= $info;
								break;
								case 1: 
									$this->fileWithInfo['image_height']= $info;
								break;	
								case 2:
									switch ($info){
										case 1:
											$type = 'GIF';
										break;
										case 2:
										 	$type = 'JPG';
										 break;
										 case 3:
										   $type = 'PNG';
										  break;
										  case 4:
										  	$type = 'SWF';
										  break;
										  case 5:
										  	$type = 'PSD';
										  break;
										  case 6:
										  	$type = 'BMP';
										  break;
										  case 7:
										   $type  = 'TIFF(intel byte order)';
										  break;
										  case 8: 
										  	$type = 'TIFF(motorola byte order)';
										  break; 
										  case 9:
										  	$type = 'JPC';
										  break; 
									}
									$this->fileWithInfo['image_type']= $type;
								break;
								case 3:
									$this->fileWithInfo['image_wxh'] = $info;
								break;
							}
						}
					}
				}
				//try to get the average info
				$this->globInfo($val);
				$this->filesWithInfo['glob_path']= $val;
				$this->files[$key]=  $this->filesWithInfo;
		}	
	}
	public function setFileProperties( $properties, $add = false){
		if( is_array($properties) ){
			if($add === false){
				$this->fileProperties = $properties;
			} else {
				$this->fileProperties = array_merge($properties, $this->fileProperties);	
			}
		}
	}
	public function  getGlobInfo(){
		$this->addGlobInfo();
		return $this->files;
	}
	public function setGlobFiles($filesArray){
		//override the $this->files array with one of our own making;
		if(is_array($filesArray)){
			$this->files=$filesArray;						
		}		
	}
	public function setGlobPath($path){
		if (!strcmp($this->path{strlen($this->path)-1}, '/' )==0 ){
			$path .= '/';
		}
		if(is_dir($path)){
			$this->path = $path;
			return true;
		} else {
			return false;	
		}
	}
	public function getGlob(){
		$this->globbers();
		return $this->files;
	}
	public function setGlobPattern($pattern){
		$this->pattern = $pattern;	
	}
	public function setGlobExt($ext){
		if(is_array($ext)){
			$this->ext = $ext;
		} else {
			//throw exception?
			die('oh smurf! I am not an array');
		}	
	}
}

Posted: Tue Mar 14, 2006 4:32 pm
by josh
I would use -> addExt() and -> remExt()... so I can add extensions one at a time, iterate it in an array outside of the class. This way you don't have to give it the entire array at once.

Posted: Tue Mar 14, 2006 4:59 pm
by neophyte
Good idea jshpro2! Will add that to the list!

Thanks