1

On many of my websites, I use a homemade PHP class called "Logger" (basically, it's intended for logging informations into log files, and organizing these files by date : year/month... automatically).

I'm using it by creating an instance of Logger in my bootstrap file (included everywhere) :

require 'lib/Logger.class.php';
$mainLogger = new Logger('./my_log_folder');

This forces me to set $mainLogger global in every function that need to log something, and also check if the logger is instantiated before call to any method :

function foo($bar){
    global $mainLogger;
    if( !is_null($mainLogger) ){
        // If the logger is instanciated, I can log my message
        $mainLogger->log('error', 'mysql-errors', "My error message lorem ipsum dolor sit amet", Logger::GRAN_MONTH);
    }
}

To make this tool easier to use and write less code, I was thinking about creating a function (outside of Logger class) that handles the instanciation and retrieval of the $mainLogger (something close to singleton design pattern) :

function getLogger(){
    global $mainLogger;
    if( !isset($mainLogger) ){
        if( class_exists('Logger') ){
            // Instanciation of the main logger
            $mainLogger = new Logger('./my_log_folder');
        } else {
            // The Logger class doesn't exists, so we'll return a magical object to "mimic" the logger attributes & methods, thus avoiding fatal errors
            return new MagicalClass();
        }
    }
    return $mainLogger;
}

class MagicalClass {
    public function __get($name){
        return;
    }
    public function __call($name, $args){
        return $this; // Allow to chain calls to this class, like jQuery : getLogger->foo()->bar()...
    }
}

The MagicalClass is intended to avoid fatal errors (Fatal error: Call to undefined method...) that could raise by calling this for example (without Logger.class.php included) :

getLogger->log('error', 'mysql-errors', "My error message lorem ipsum dolor sit amet", Logger::GRAN_MONTH);

Thanks to _call and _get, any attempt to use attributes or methods of the Logger class will not cause any error (error logging is an optional feature, it shouldn't crash the app simply if the Logger doesn't exists).

What do you think about this approach, is it a bad idea ? Can this lead me to some troubles, and what kind ?

Thanks

PS : If you want to see the Logger class, you can download it on my website here.

Vince
  • 3,274
  • 2
  • 26
  • 28
  • Fatal errors shouldn't happen in production code. Fatal errors are usually very serious run-time errors, or worse, syntax errors (Syntax errors are compile-time, they cannot be logged by any programmed mean, and are always fatal). – Madara's Ghost Oct 10 '12 at 21:01
  • The answer by JvdBerg is correct in my opinion. Might be a quick fix though to use a common base class that contains the logging functionality? You could use a lazy load method in a base class that instantiates the logger on first call. I wouldn't use the magic in this case, magic = mystery. And in a code base I think mystery is bad. In this case you need to deal with the fact the logging may not be present, so make it present it all objects that might call upon it. – Gavin Oct 10 '12 at 21:06
  • 2
    This is not a good way of handling errors and there are many fatal errors that can't be avoided (e.g syntax). Using a class like this throughout your code-base couples your code tightly to your logging implementation. [See my answer here](http://stackoverflow.com/questions/10331084/error-logging-in-a-smooth-way/10538836#10538836) for a detailed description of error handling options. – Paul Oct 11 '12 at 02:27
  • @Paul excellent solution to error logging. One of the best answers I have read on SO! Kudos. Vince is your requirement about logging errors, building trace, or both? – Gavin Oct 11 '12 at 07:19
  • @Paul I agree, but SQL error logging was just an example. In fact it's not the main purpose, I'm using my Logger class to record many things like search engines crawlers visits, web analytics related things (in addition to Google Analytics's event tracking) or security monitoring (like session hijacking attempts). – Vince Oct 12 '12 at 08:12

2 Answers2

4

The 'problem' you are describing here can be solved in a number of ways. The 3 most common are:

  1. Use of a factory class. A factory class is a object generating factory. The factory itself is a static or a singleton class, globaly used within a class.

  2. Use of Dependency Injection. With this technique the logger class is injected in the constructor of a class. The class keeps a reference to the logger for later use.

  3. Use of Inversion of Control (IoC) container. This is a combination of 1 and 2. The container keeps is a list created objects, and when a new object is needed it is created and dependant objects are automatic injected in the constructor.

Examples:

Factory class

class Foo
{
  public function Bar()
  {
    $logger = ClassFactory::CreateLogger();

    $logger->log('error', 'mysql-errors', 
      "My error message lorem ipsum dolor sit amet", Logger::GRAN_MONTH);
  }
}

Dependency Injection

class Foo
{
  private $logger;

  public function __construct($logger)
  {
    $this->logger = $logger;
  }

  public function Bar()
  {
    $this->logger->log('error', 'mysql-errors', 
      "My error message lorem ipsum dolor sit amet", Logger::GRAN_MONTH);
  }
}
JvdBerg
  • 21,777
  • 8
  • 38
  • 55
1

I do not believe you need this magic class.

First, this magic class does not implement the Logger, so typehinting (which is a good thing) will not work. Second, because of your magic class not implementing or extending Logger, any IDE will also not show autocompletion on the magic class. Bad thing indeed.

You would only need the magic class if the Logger was not loaded before. This is so basic failure that it should be very easy to detect. The simplest way would be to include the Logger in the file that now has the magic class.

On the other hand, how do you guarantee that your function getLogger() is available? If the Logger might be unavailable, the function might be the same, and your code will equally fail.

Code improvements: Do not use this:

function getLogger(){
    global $mainLogger;

You do not need a global variable, you only need a variable that stores your logger for later retrieval. Use a static variable instead:

function getLogger(){
    static $mainLogger;

If you are at it, this function could go directly into a factory class and be called statically. The variable $mainLogger will then be a static property of the class, probably in private scope.

Sven
  • 69,403
  • 10
  • 107
  • 109
  • getLogger will always be available because I'll write it in the core library file of my app. I had not thought of using static, thanks for that tip. – Vince Oct 12 '12 at 08:17
  • That's where your logger should be required then, too. Problem solved, no need for magic. – Sven Oct 12 '12 at 08:18