Can I use static class for my Logger?

问题:

Recently I have been told that static class/methods are evil.

Take for example my class Logger:

class Logger{
   private static $logs = array();
   public static function add($msg){
      self::$logs[]=$msg;
   }

   public static function echo(){
       print_r(self::$logs);
   }
}

I can use whenever i want in my appliaction like this:

Logger::add('My log 1');

But reading this developers:

• http://misko.hevery.com/2008/12/15/static-methods-are-death-to-testability/

That Logger class doesn't seem so good.

So: Can I use it statically or I should avoid it at any cost?

回答1:

Logging classes are the exception.

Since they rarely contain much logic, you don't have the same testing concerns.

Logging is a perfect example of a GOOD place to use static classes.

Think of your alternatives:

• A global instance of a logging object?
• A singleton logging object?
• Pass the logging object around to every single method/class (via in a constructor)?

The above are much worse than using static for logging.

回答2:

Avoid it. I've seen quite some posts of you now struggling with the issue and people giving you bad advice. I'll repeat what I said in some of my answers/comments.

The way you use static in your logger class is to use it as a globally access point. Whenever you need to logg something you call Logger::log().

1) You will not be able to tell from looking at your class definition that it depends on the Logger class. Change in code thus becomes an adventure: 'I hope I will not break some hidden dependency when I change this tiny little ... OOPS!'.

2) It IS harder to test. You can't realiably test a class that sends a message to the Logger with Logger::log(). When a test fails how will you know it is not because the Logger fails? You would know if you could replace it with a mock, but in your case it is not mockable.

An alternative to explore:

Use the observer pattern and make the Logger an observer, the classes that need logging can be observables. They send messages like $this->observers->nofify('test succeeded').

You could use some other form of events too or dependency injection (automatic or manual). But please please don't call Logger::log() in a method.

回答3:

I still think that logging is a valid approach to use a static classes. The often stated phrase that it is not testable is imho also not true if you do it right. I want to implement this but did not find the time, however, I thought about something like the following.

class Logger {

    protected static $handlerSet = [];

    // Pure static class {{{
        private function __construct() {}
        private function __clone() {}
        private function __sleep() {}
        private function __wakeup() {}
    // }}}

    public static function critical($message, array $context = []) {}

    // You know the PSR drill...

    private static function log($level, $message, array $context) {
        foreach ($this->handlerSet as $handler) {
            $handler->handle($level, $message, $context);
        }
    }

}

Of course we do not want to expose the management of the handlers to all classes, hence, we use a child class that has access to the protected handler set.

final class LoggingManager extends Logger {

    public static function addHandler(Handler $handler, $name, $level) {
        static::$handlerSet[$name] = $handler;
    }

    public static function removeHandler($name) {
        if (isset(static::$handlerSet[$name])) {
            unset(static::$handlerSet[$name]);
        }
    }

    public static function resetHandlers() {
        static::$handlerSet = [];
    }

    // Other useful stuff...

}

Testing is now fairly easy, if you actually want to test something like logging (could be that it has some ROI for you, don’t know).

class SomeTest extends YourFrameworksTestCase {

    public function testThatSomethingLogsSomething() {
        try {
            $handler = new TestLogHandler();
            LoggingManager::registerHandler($handler, 'test', 'debug');
            // Test something.
            $this->assertLogRecordExists($handler, '[debug] StackOverflow');
        }
        finally {
            LoggingManager::resetHandlers();
        }
    }

}

It would also be possible to create a more sophisticated test case to extend that implements all of the log record assertion for you. This approach is imho fairly easy and a class in your system should not care whether a handler is registered or not, nor what it does with the logged messages. Things like that are handled in your application and only there. The advantages are obvious:

• Global access to the logger and logging manager.
• Easy testing, comparable to dependency injection.
• No need for code polluting DIC solutions.
• Single logger instance, always.
• …

回答4:

While there is nothing wrong with that approach, I recently moved from a static logging class approach to log4php in one of my own projects myself.

log4php uses a separate instance of a logging class for each class in your project. When looking at that logging framework, the benefits become obvious.

Logged messages always have a context (the class through which the message was logged). That allows for easy filtering (and make the log slightly more helpful).

回答5:

The only problem with static classes is that they are hard to change. So it's ok here since you're class doesn't do much.