[
https://issues.apache.org/jira/browse/LOG4J2-1093?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14721622#comment-14721622
]
Bart S. commented on LOG4J2-1093:
---------------------------------
Why should you see the code actually working if you are not interested in it? I
was offering it as a thing I can complete if the interest is there, not as a
thing to waste my time before you decide there is an interest or not. I'm not
your slave :-/.
The idea is pretty clear and I told you I changed non-builder methods (notably,
create......()) in order to separate sanity checks from those two construction
methods. There is a creat.....() paradigm that uses a static create.....() call
and there is the builder paradigm but both have to do sanity checks in
principle. In ConsoleAppender that doesn't happen.
So it was more like a question and a proof of concept. The question was whether
you'd find it ugly and whether the builder would be welcome in the first place,
I know there an ongoing discussion (that was started by me, coincidentally) and
I too would hold back with making changes like this. Nevertheless you could
shelve it, but I still require feedback. You still haven't given it (neither
has Ralph, who just does what he wants without regards for others).
I would envision that a good solution for that ongoing discussion would indeed
void some of the builder instances (provided that the regular config options
using files/scripts do not use any builder paradigm at all (they use strings I
believe as does the Create method, so I believe they all use create methods).
It is a big infrastructure and Ralph is ruining it with his Assembler solution
that he hasn't thought about well enough and he disfavours discussion and
thinking about it or reflecting on it, rather just throwing together a solution
that must work and then must be everyone's solution.
Yes I'm angry but I'm also just disgusted, sorry about that.
And I don't even know about plugins, I don't know what they are or how they are
used except that they use annotations; that I copied from the other Appender
;-) without understanding why or how. So I cannot really even implement
something like this without studying plugins first, which I not wanted to do if
there was not or was not going to be an interest in the first place. I'm just
not going to waste time doing stuff that is disregarded, as if you can choose
and pick what another makes without regard for the time or effort of that
other..... :-/. (I guess that is the Git workflow: every contributor is a
server and there is like one client (or two, or three) that pulls commits from
the server. In this case we supply patches via JIRA which is better.
For me, at least.
So I already told you that the patch contains non-builder changes, you don't
have to be surprised about that if you read what I wrote. Coding takes a long
time and writing unit tests even more especially as I'm not very much used to
the framework.
I just wrote a suite of unit tests in Bash for a library I wrote and it was
very handy indeed and much nice than JUnit, but I barely understand JUnit so
yeah. It just seems rather impossible to create any form of easy "story" in
there. Everything has to be its own loose thing. Which means you'd rather write
a big application to test everything rather than anything else. (In Bash you
run the constant risk of regressions all the time, so having a unit test there
is quite essential).
I got quite in the habit of rerunning my test cases after every change :).
Saved my a life a hundred of times. :D.
The real magic of this builder is not whether it works, but if you want it in
the first place. Of course it works, there is nothing special about that. You
also will become none the wiser if you do see it work, as if this is some for
of new magical creation that still needs to be demonstrated in its
principles.....
So you will gain nothing by seeing a unit test at this point.
> 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: [email protected]
For additional commands, e-mail: [email protected]
