[
https://issues.apache.org/jira/browse/LOG4PHP-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12845084#action_12845084
]
Ivan Habunek commented on LOG4PHP-102:
--------------------------------------
I am contributing two possible solutions to this problem (both fix the above
issue):
The first solution is to rewrite the code to use PHP string manipulation
methods, as stated in a @todo. This makes the spacePad() method obsolete (and
with it the global variable holding spaces) and so fixes the problem. Also, the
code is much shorter and easier to read. However, I did some basic testing, and
it seems that the str_pad method is not as fast as the spacePad() method. This
patch can be found in LoggerPatternConverter.patch.rewrite.txt
The second solution is to use a local member variable to hold the spaces array
instead of a global variable (it is not used anywhere else in the project).
This solution has no performance impact. The patch can be found in
LoggerPatternConverter.patch.quickfix.txt
Personally, I would prefer to use the first solution, because it is neater, but
I don't have a good way to measure how this will affect overall performance of
log4php. Can any of the project developers take a look at this issue?
I am also contributing a new set of tests, because the ones currently in
LoggerLayoutPatternTest class are a bit thin. The new tests include a more
complicated pattern (as requested in a TODO) and testing of all log levels. The
patch can be found in LoggerLayoutPatternTest.patch.txt
> LoggerLayoutPattern fails tests
> -------------------------------
>
> Key: LOG4PHP-102
> URL: https://issues.apache.org/jira/browse/LOG4PHP-102
> Project: Log4php
> Issue Type: Bug
> Components: Code
> Affects Versions: 2.0
> Reporter: Ivan Habunek
> Priority: Minor
> Attachments: LoggerLayoutPatternTest.patch.txt,
> LoggerPatternConverter.patch.quickfix.txt,
> LoggerPatternConverter.patch.rewrite.txt
>
>
> In LoggerLayoutPatternTest class, the testWarnLayout() method is commented
> out because it fails with the following message:
> 1) testWarnLayout(LoggerLayoutPatternTest)
> Undefined index: log4php.LoggerPatternConverter.spaces
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/helpers/LoggerPatternConverter.php:131
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/helpers/LoggerPatternConverter.php:104
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/main/php/layouts/LoggerPatternLayout.php:216
> /Users/cgrobmeier/Documents/Development/workspace/log4php-trunk/src/test/php/layouts/LoggerLayoutPatternTest.php:45
> This seems to be related to using a global variable
> $GLOBALS['log4php.LoggerPatternConverter.spaces'] in
> LoggerPatternConverter::spacePad(). I am unsure why the global variable is
> not accessible in this test, but using a global variable seems the wrong
> approach here anyway.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
