logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bart S. (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (LOG4J2-1093) Builder for FileAppender
Date Mon, 31 Aug 2015 12:10:46 GMT

    [ https://issues.apache.org/jira/browse/LOG4J2-1093?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14723340#comment-14723340
] 

Bart S. edited comment on LOG4J2-1093 at 8/31/15 12:10 PM:
-----------------------------------------------------------

Discussion is NOT out of question for me, that's the whole point. Discussion is what I'm asking
for. Discussion is what I'm not getting. Discussion is mandatory for making things a complete
package (with unit tests and all) because it's still in the early stages (and I hate writing
JUnit tests at this point). And I don't need to be taken seriously by people who cannot be
taken seriously themselves.


was (Author: xennex82):
Discussion is NOT out of question for me, that's the whole point. Discussioln is what I'm
asking for. Discussion is what I'm not getting. Discussion is mandatory for making things
a complete package (with unit tests and all) because it's still in the early stages (and I
hate writing JUnit tests at this point). And I don't need to be taken seriously by people
who cannot be taken seriously themselves.

> 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.
>              Labels: features, patch
>         Attachments: FileAppender_patch-Builder-proposal.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> 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: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message