Marshall Pierce wrote:
Over the past little while, I've been working here and there on
improving log4php to use PHP 5 features for my employer, and I've gotten
the OK to contribute my improvements back to the project. I'm not quite
finished with my improvements, but I wanted to get some feedback as I
continue forward.
Improvements:
- Relocate and rename classes to support autoloading (and provide an
autoload implementation)
- Move from php4 'var' to private/protected as appropriate and add
getters/setters as needed
- Bug fixes / other minor refactorings
- Use exceptions (still remaining to be done: make sure exceptions will
not bubble up into code that uses log4php)
- Reformat code (mostly -- there are still parts that I haven't gotten
to yet) to match the standard we use at work
Thanks for sharing your fork of the project.
log4php already works with PHP 5:
http://svn.apache.org/repos/asf/incubator/log4php/trunk/src/main/php/
and we have a bunch of working examples:
http://svn.apache.org/repos/asf/incubator/log4php/trunk/src/examples/php/
and unit tests (using PHPUnit 3.2):
http://svn.apache.org/repos/asf/incubator/log4php/trunk/src/test/php/
The PHP 5 version of log4php was the reason why the whole incubation
process was restarted and I used all the examples and the unit tests to
hunt down bugs.
I prefer to stick with the current naming convention and focus on the
functionality. When I started working on the PHP 5 version I did the
same thing as you did and ended up with the same directory structure and
naming convention. But after a while I realized that I was just wasting
hours for no good reason and would break existing applications, so I
started focusing on the stuff that mattered.
You mention that you have done various bug fixes. It's really
appreciated if you can file patches against the current code base. While
writing the PHP 5 version I fixed a whole lot of minor things. I'm
pretty sure you have find some new, and that many of them are already fixed.
About new functionality we already have a lot of stuff on our TODO-list:
http://svn.apache.org/repos/asf/incubator/log4php/trunk/TODO
I see that one of the reasons behind your fork is the autoload
mechanism. We should provide an autoload method in log4php, but this
could be done with a associative class mapping array, where the
classname is the key and the relative file path is the value. That is a
better solution of two reasons:
1) log4php can provide an autoload method with the current naming convention
2) The performance is much better because it's simply a lookup in an
array instead of IO-access and checking if the class file that are to be
loaded actually exists, which is unnecessary. (log4* should be as
lightweight as possible so this is important)
Best regards,
Knut Urdalen
