2

I have a simple importer class that logs success and failure statuses to a log file.

I have made the log file name a constant in the class like so:

class MyClass
{
    const STATUS_LOG = "my_log.log";

    public function doImport()
    {
         // do import here and log result
    }
}

Currently i know of no reason that different logs would be used, but would it be better to allow that flexibility and do the following instead:

class MyClass
{
    private $statusLog;

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

    public function getStatus()
    {
        return $this->statusLog;
    }

    public function setStatusLog($statusLog)
    {
        $this->statusLog = $statusLog;
    }

    public function doImport()
    {
         // do import here and log result
    }
}

Given i currently have no use for different log files, is there any benefit in the second approach?

4

4 回答 4

2

I think that in terms of logging you should not allow to change log path. Not in runtime - since there's a question - what will happen with data integrity if log path will changed 'on hot'? Flexibility sounds good, but I think this is not a case when you should allow change your property in runtime.

If you're hesitating about log path then it should be adjustable via configuration file - i.e. read once at application's start. So you will not store path in your class, reading it from config instead (in __construct() for your class).

于 2013-08-29T12:39:58.973 回答
0

If you have a static logfile path that doesn't change you can stay at the first one.

If it should be able to change the log path (weather now or in planned future) I would use the second one. (The benefit is just to be able to set and get the logpath, a getter can also be used for the firse solution).

于 2013-08-29T12:36:10.110 回答
0

Your class might violate the single responsibility principle: from the doImport method I conclude that the first responsibility of your class is importing and it should not concern itself with the details of logging (formatting, what file to use, what to log in development/production environment). Why not pass in a logger class (preferably a class that implements the PSR-3 interface) in the constructor? Monolog is a fantastic and flexible logging library.

If you keep the filename (or better the logger) flexible, you can write unit tests for your class without having to worry about overwriting important files. If you use a memory/dummy logger class, you don't even need a file system!

If you just write a small one-off script that will not be tested however, then using a constant is a good way for configuration IHMO.

于 2013-08-29T12:47:33.663 回答
0

This is a no-brainer. The second approach is more flexible and more explicit. Also, the log file name is not a "real" constant in the sense that, say, pi or equator length are. When defined as constant, it's just a "magic value" you'll sooner or later forget that it's there.

I'd get rid of the setter method though and only allow setting the value in constructor.

于 2013-08-29T12:47:33.913 回答