Critique my other code

[Andy-H]

I started another topic asking for help with a class I had written and was wondering if anyone would be kind enough to point out bad practices/improvements etc. on this class I wrote quite a while back, any help greatly appreciated.

 

 

<?php
class parseCSV
{
   protected        $_fh;
   protected        $_fs;
   protected        $_fn;
   protected static $_contents;
   protected static $_delimiter;
   protected static $_escape;
   protected        $_data = array();
    
   public function __construct($filename, $delimiter = ',', $escape = '\\')
   {
      ini_set('auto_detect_line_endings', 1);
      if ( !file_exists($filename) )
         throw new Exception($filename . ' does not exist, please check the filename for typos.');
         
      $fh = fopen($filename, 'r+');
      if ( !$fh )
         throw new Exception('File ' . $filename . ' could not be read. Please try again.');
      $this->_fn        = $filename;
      $this->_fh        = $fh;
      $this->_fs        = filesize($filename);
      self::$_delimiter = $delimiter;
      self::$_escape    = $escape;
   }
    
   public function read()
   {
      while( $cols = fgetcsv($this->_fh, $this->_max_line_length(), self::$_delimiter, '"', self::$_escape) )
      {
         $cols = array_filter($cols);
         if ( empty($cols) ) continue;


         $this->_data[] = $cols;
      }
   }
   public function write($fields)
   {
      $fh = fopen($this->_fn, 'w');
      foreach($fields as $field)
      {
         fputcsv($fh, $field, self::$_delimiter, '"');
      }
   }
   public function get_data()
   {
      return $this->_data;
   }
    
   protected function _max_line_length()
   {
      $this->_get_contents();
      $lines = explode("\r\n", self::$_contents);


      return max(array_map('strlen', $lines));
   }
   protected function _get_enc()
   {
      $this->_get_contents();
      $enc = mb_detect_encoding(self::$_contents, 'auto', true);


      return ( $enc != false ) ? false : 'UTF-8';
   }
   protected function _get_contents()
   {
      if ( !self::$_contents )
      {
         self::$_contents = fread($this->_fh, $this->_fs);
         rewind($this->_fh);
      }
   }
   public function close()
   {
      fclose($this->_fh);
   }
}
?>

 

 

Thanks

[Andy-H]

Just to explain myself a little, looking at the code I think I used static because I didn't understand it properly, from my current understanding static will be consistent through all instances of the object, my previous (mis)understanding was that static was based on the instance, and just couldn't be updated after it had been set? Since per object is per-csv I though this would prevent the delimiter and escape character from being changed. If this is correct, there's one improvement

 

 

Also I see that I should change the content member variable to not be static, this means I could update the contents on a write to the file, and maybe implement a way to change the file that it acts upon?

 

 

Thanks for any help.

[Andy-H]

Just realised that this code really sucks, gonna re-write it later to extend a base class for file handling, will post back when I'm done.

[Andy-H]

 

http://uk3.php.net/manual/en/class.splfileobject.php

 

Never mind lol