PHP how could I make this class better? Suggestions/Feedback welcome

Question

Best suggestion will be the accepted answer

Class:

<?php 

class LogInfo {
    private $first_run;         // Flag to add line break at the beginning of script execution
    private $calling_script;    // Base name of the calling script
    private $log_file;          // log file path and name
    private $log_entry;         // information to be logged
    private $log_level;         // Log severity levels: error, warning, notice, info (default)
    private $log_level_arr;     // Array of accepted log levels
    private $fh;                // File handle

    private $file_name;         // File path and name
    private $file_parts;        // Array of $file_name
    private $script_name;       // Script Name
    private $script_parts;      // Array of $script_name    

    function __construct() {
        $this->first_run        = true;
        $this->log_level_arr    = array(
                                    'info'=>'info', // default
                                    'error'=>'error',
                                    'warning'=>'warning',
                                    'notice'=>'notice',
                                    );
        $this->calling_script   = '';       
        $this->log_file         = '';
        $this->log_entry        = '';
        $this->log_level        = '';
        $this->fh               = '';
        $this->file_name        = '';
        $this->file_parts       = '';
        $this->script_name      = '';
        $this->script_parts     = '';                   
    }

    /**
     * log detailed information into calling script log files
     */
    public function logInfo($info, $level = 'info') {

        if(array_key_exists($level,$this->log_level_arr)) {
            $this->log_level = $level;
        } else {
            $this->log_level = 'undefined';
        }

        $this->calling_script = $this->getScriptBaseName();
        $this->log_file = dirname(__FILE__)."/logs/".$this->calling_script.".log";
        $this->fh = fopen($this->log_file, 'a') or die("Can't open log file: ".$this->log_file);

        if($this->first_run) {
            $this->log_entry = "\n[" . date("Y-m-d H:i:s", mktime()) . "][" .$this->log_level."]:\t".$info."\n";
        } else {
            $this->log_entry = "[" . date("Y-m-d H:i:s", mktime()) . "][" .$this->log_level."]:\t".$info."\n";
        }

        fwrite($this->fh, $this->log_entry);
        fclose($this->fh);

        $this->first_run = false;
    }

    /**
     * return the base name of the calling script
     */
    private function getScriptBaseName() {
        $this->file_name    = $_SERVER["SCRIPT_NAME"];
        $this->file_parts   = explode('/', $this->file_name);
        $this->script_name  = $this->file_parts[count($this->file_parts) - 1];
        $this->script_parts = explode('.', $this->script_name);

        return $this->script_parts[0];
    }
}

?>

How to call:

$log = new LogInfo();

$txt = "This is comment";
$log->logInfo($txt." 1",'error');
$log->logInfo($txt." 2",'new');
$log->logInfo($txt." 3",'warning');
$log->logInfo($txt." 4",'notice');
$log->logInfo($txt." 5",'info');
$log->logInfo($txt." 6");

Answer 1

I would use separate methods for each log level, that would be easier to use.

$log->error($msg) for example.

Answer 2

As jishi already stated, I would also separate the log-message-levels into own methods. These log-methods would be protected and only get invoked through one log-method which is public to use for the programmer.

Also I would add the possibility to add different types of writers. So that you can log the data into a file, database, send via email, and so on. I would realize this with dependecy-injection. If you need an example, about what I'm thinking of, just say it :)

EDIT

I'm home right now, and as promised I'm gonna provide some code:

/**
 * This is an interface for the writers. All writers must implement this,
 * to ensure, that they work together with the logger-class.
 */
interface logWriter
{
    public function log($message, $type);
}

/**
 * This is your logger-class. You call this class, if you want to log something,
 * just as you do it now.
 */
class logger
{
    /** This is the most important var. All writers that shall be used, are registered
     * in this array.
     */
     protected $_writers = array();

     // Here the other vars go, that you need for your logger, like config-vars and stuff
     // ...
     // ...

     /**
      * Your constructor, probably you'll use it to configure the logger...
      */
      public function __construct($config)
      {
        //.... do stuff...
      }

      /**
       * Okay with this method you register the different writers with your logger
       */
       public function registerWriter(logWriter $writer)
       {
            // Store the write into the array with all writers.
            $this->_writers[] = $writer;
       }

       /**
        * This is your normal log-method, just as you have one now
        */
       public function log($message, $type)
       {
            // Now you iterate over all registered writers and call the log-method of the writer...
            foreach($this->_writers AS $writer)
            {
                $writer->log($message, $type);
            }
       }
}

/**
 * This is a writer that sends the log message via email to you
 */
class emailWriter implements logWriter
{
    /**
     * Implement log-method from the logWriter-interface
     */
     public function log($message, $type)
     {
        // This is simplified right now. Should be more complex in the final version
        $to = '[email protected]';
        $from = '[email protected]';
        $subject = 'Log-Message of type' . $type;
        $body = $message;
        $headers = $from . "\r\n";
        mail($to, $subject, $body, $headers);
     }
}

/**
 * This is a writer that writes the message into a log-file
 */
class fileWriter implements logWriter
{
    /**
     * Implement log-method from the logWriter-interface
     */
     public function log($message, $type)
     {
        //... Here you just do the stuff that you would do: open file, write content, close, etc...
     }
}

// ...
// Here you can develop more writers like an SMS-Writer, Jabber-IM-Writer and so on
// ...

Okay now using this stuff:

$logger = new logger();

// Register the writers with the logger
$logger->registerWriter(new emailWriter());
$logger->registerWriter(new fileWriter());

// Now log something
$logger->log('This is a test-log-message', 'info');

Since we've registered the email- and fileWriter to the logger, the message will be written into a file andadditionally you'll get an email.If you only want to get an email and don't write the message into a file, only register the emailWriter with the logger-class

Answer 3

I will often add some static functions to a logger to make it more easily accessible without an instantiated object. Building on jishi's suggestion of different methods for each level:

public static function info($message) {
   // do logging here
}

LogInfo::info("I can log from anywhere");

Of course, there are certainly times and places where you don't want to access your logger using static functions. I generally find it easier though.

Another suggestion is to read in a configuration variable from somewhere that specifies a logging level. When you're building and very curious about exactly what is happening where, you can turn your logging level up from a central location (i.e. config.php or something).

Change your main log method to check that config variable before doing anything...

public static function info($message, $logLevel) {
    if(check_config("logLevel") >= $logLevel) {
        // do logging here
    }
}

Answer 4

You may want to make this class implement an observable interface so you can attach it to other objects which can then use it to log events using the Observer pattern.

interface Observer
{
    function notify($sender);
}

interface Observable
{
    function addObserver(Observer $observer);
}

class LogInfo implements Observer
{
    // ...

    public function notify($obj)
    {
        $this->logInfo($obj->getMessage());
    }

    // ...
}

class DoesSomething implements Observable
{
    // ...

    protected $observers = array();
    protected $message;

    public function addObserver(Observer $observer)
    {
        $this->observers[] = $observer;
    }

    protected function notify($message)
    {
        $this->message = $message;
        if (count($this->observers))
        {
            foreach($this->observers as $observer)
            {
                $observer->notify($this);
            }
        }
    }

    public function getMessage()
    {
        return $this->message;
    }

    public function doSomething()
    {
         // ...
         $this->notify('We did something important');
    }

    // ...
}

$obj = new DoesSomething();
$obj ->addObserver(new LogInfo());
$obj ->doSomething(); // Uses LogInfo object to log event

Answer 5

Very similar question from earlier this week: http://stackoverflow.com/questions/3187219/how-do-i-save-logs-in-php/3187389#3187389