views:

95

answers:

4

I'm writing a logging tool which writes messages to a file, and I'm not sure of the best way to deal with the file pointer. I'm tossing up between these two methods:

// Open, Write, Close; Open, Write, Close...
function write($message) {
    $fh = fopen('file.log', 'a');
    fwrite($fh, $message . "\n");
    fclose($fh);
}

// OR -----

// Open, Write, Write, Write..., Close
function __construct() {
    $this->fh = fopen('file.log', 'a');
}
function __destruct() {
    fclose($this->fh);
}
function write($message) {
    fwrite($fh, $message . "\n");
}

I imagine that this class will be loaded and constructed on every page, but not necessarily used, though it most likely will be.

Are there are performance, security or miscellaneous pitfalls of either method, and which would you recommend?

+2  A: 

I can't give you a number, but opening a file just to close and reopen it repeatedly is going to cost you something. And I'm talking at the OS level, not just PHP. Calling into the kernel for open() (or CreateFile() if you speak Windows), matching path strings to entities from your filesystem, reading directory structure from disk, then another syscall to close the handle... Especially if you have a large number of this I would avoid doing all this too much. Or get some measurements to see if it's OK.

If you're worried about incurring cost for something that will never be used, have you considered a mix of these two approaches? That is, doing an open on write if the file isn't already open, otherwise reusing the old handle...

asveikau
you mean singleton.
lemon
"you say tomato...." I am aware of patterns, but I'm not one of these types who prefers jargon over actually describing the techniques. :-)
asveikau
+1  A: 

Depends on what you're logging.

Really the second version is best. Think of it like this:

You want to log errors that occur during the requested cycle. Let's say you get 3 errors during the request. If you use the first method you are opening and closing the file 3 times which causes more overhead. If you are using the same instance of the logger class you are only opening the file once and writing 3 lines to it during the request.

Hopefully this clears things up for you.

Optionally you can pass a file pointer to the first method and only have the method perform the write.

Derek Reynolds
+1  A: 

Here is an alternate version:

function log($message) {
  global $_log;
  if (!defined(LOG_OPEN)) {
    define(LOG_OPEN, true);
    $_log = fopen('filename.log', 'w');
    register_shutdown_function('close_log');
  }
  fwrite($_log, $message . "\n");
}

function close_log() {
  global $_log;
  fclose($_log);
}

At least then you're only opening the log when you write a message.

It will cost you something. In the grand scheme of network latency costs and whatnot I doubt it'll be significant unless you start opening lots of files. I doubt this one log would even be measurable in its effect.

cletus
This looks nice. I would consider using a static variable instead of a global to help with encapsulation though with the shutdown function calling log with an extra parameter to close the file
Stewart Robinson
+1  A: 

The second method is obviously better, IMO. Opening and closing a file handle does have a cost, and although it's relatively minor, it does add up to quite a bit if you do this a lot within the application (although if it's a very rare thing, it won't make a difference I don't think). And if you ever need to add some extra setup or cleanup functionality, the second method would be the cleanest, because you just add some code to the __construct/__destruct method.

musicfreak