Jump to content
#StayAtHome ×

Archived

This topic is now archived and is closed to further replies.

Andy-H

Critique my other code

Recommended Posts

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 :)

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.