Bart S. created LOG4J2-1093:
-------------------------------

             Summary: Builder for FileAppender 
                 Key: LOG4J2-1093
                 URL: https://issues.apache.org/jira/browse/LOG4J2-1093
             Project: Log4j 2
          Issue Type: Improvement
          Components: Appenders
    Affects Versions: 2.4
         Environment: Any
            Reporter: Bart S.


Hi, In my quest for programmatic control I ran into the fact that the 
FileAppender didn't have an Builder method to use.

I really wanted to brainstorm about it first before I sent in my patch, seeing 
as that it might deviate from coding standards that I'm not completely aware of.

The FileAppender has these issues with creating a Builder:

- the create..... method takes String parameters and does sanity checking
- to have a builder use the create.... method requires converting all 
parameters back to String, which is ugly and in a sense time consuming.
- the same is true for the ConsoleAppender and its builder just does away with 
any sanity checks and error messages and just calls the constructor directly.

I have taken a different path in my code. I have extracted the sanity check and 
placed it in its own routine. So there are basically four options:
* no sanity checking for the builder
* builder uses create method
* create method uses builder
* separate sanity checking and call both from create and builder

I have currently chosen the latter path. I use a MutableBoolean or the 
equivalent. My current code uses a new static inner class just thrown together 
for this purpose, but I believe the Core utilizes org.apache.commons.lang3, 
which contains mutable.MutableBoolean.

The rewrite is currently complete and compiles well. The unit test for 
FileAppender still completes without fail (1 skipped, it says):

{quote}
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.logging.log4j.core.appender.FileAppenderTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 4.713 sec - in 
org.apache.logging.log4j.core.appender.FileAppenderTest

Results :

Tests run: 6, Failures: 0, Errors: 0, Skipped: 1

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:18 min
[INFO] Finished at: 2015-08-12T17:59:18+02:00
[INFO] Final Memory: 22M/336M
[INFO] ------------------------------------------------------------------------
{quote}

Meanwhile, my own code that utilizes it compiles against it and works 
perfectly. I haven't written a unit test or tests for it yet.

Moreover, I have made these changes:

- extract sanity checking from create........() method. Into its own 
"checkParameters() : boolean".
- the boolean value that is subject to being changed is turned into a type of 
MutableBoolean (simply a class with a boolean field at this point)
- this variable is passed to the checking function where it might get changed
- the value of the variable is subsequently used in place of the original 
boolean

- this {{checkParameters()}} is called from {{createAppender()}} AND from 
{{Builder.build()}}.
- a minor amount of code is still duplicated, namely the creation of the 
manager (FileManager) and the call to the constructor.

- annotations and style is/are mimicked as much as possible from 
ConsoleAppender.Builder

That's all for it now. You can view the result at the attached diff. Note that 
there is still no unit test for it and this is just a proposal, open for 
amendment and suggestion.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to