logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: Usage of generics in Appender
Date Wed, 14 Aug 2013 14:37:46 GMT
On Wed, Aug 14, 2013 at 9:52 AM, Remko Popma <remko.popma@gmail.com> wrote:

> Gary,
>
> The change only affects Appender, Layout is still generic.
> Appender does not need to be generic (it can just return a Layout<?
> extends Serializable> from its getLayout() method).
>
> This change does not introduce any type casts (afaics).
>

This might be academic, but it would have to introduce type casts since we
have two kinds of layouts: String and LogEvent.

For example:

        LogEvent logEvent = null;
        logEvent = new SerializedLayout().toSerializable(logEvent);
        String xmlFragment = XMLLayout.createLayout(null, null, null, null,
null, null).toSerializable(logEvent);

Gary



> Remko
>
>
>
> On Wed, Aug 14, 2013 at 10:11 PM, Gary Gregory <garydgregory@gmail.com>wrote:
>
>> On Wed, Aug 14, 2013 at 4:32 AM, Remko Popma <remko.popma@gmail.com>wrote:
>>
>>> Patch looks nice. Much fewer "raw type" compiler warnings. Seems like a
>>> big improvement to me.
>>>
>>> A small improvement on the patch would be to remove the (now
>>> unnecessary) @param <T> javadoc comments. I have that additional change
>>> done in my local workspace.
>>>
>>> I wouldn't mind committing this but I don't want to disrupt anyone's
>>> work-in-progress: the patch modifies about 125 files.
>>>
>>> Is everyone ok with me committing this? I'll hold off for a day or two
>>> (or less if we're all ok with this).
>>>
>>
>> Hi All:
>>
>> Please also consider these thought paths:
>>
>> - The raw compiler warnings can also be considered as a 'task not
>> finished' warning. That is, I did not go all the way in pushing generics
>> through the whole code base.
>>
>> - Consider the Appender/Layout generics separately from the other
>> generics in the code. I think this is already the case based on other
>> messages I've read, but I wanted to make sure 'the baby does not get thrown
>> out with the bath water' (a goofy saying here in the US).
>>
>> - Weigh was is 'correct' vs. what is 'convenient' vs. compiler warnings.
>>
>> - What is the goal? For me, the goal of the generics here are multiple:
>>   -- Have the ability to write code that, for example, can configure and
>> start Log4j and then manipulate it without type casts.
>>   -- Avoid type casts in general.
>>   -- Cause compile time errors instead of a runtime errors when extending
>> Log4j.
>>
>> Gary
>>
>>
>>>
>>> Remko
>>>
>>>
>>> On Monday, August 12, 2013, Ralph Goers wrote:
>>>
>>>> I've briefly glanced at your patch and it looks reasonable to me.  It
>>>> leaves the Layout to continue to use generics but removes the generics from
>>>> the Appenders.  That seems like a reasonable thing to do.
>>>>
>>>> Ralph
>>>>
>>>> On Aug 11, 2013, at 6:34 PM, Henning Schmiedehausen wrote:
>>>>
>>>> Ok, that makes a lot of sense. I have reworked the patch to leave these
>>>> alone and put it on a different git branch. I also opened a ticket:
>>>> https://issues.apache.org/jira/browse/LOG4J2-343 and attached it as a
>>>> patch so that if you do not want to deal with git, you can get it from
>>>> there.
>>>>
>>>> I added instructions on how to get the patch from git to the ticket.
>>>>
>>>>
>>>> On Sat, Aug 10, 2013 at 4:40 PM, Nick Williams <
>>>> nicholas@nicholaswilliams.net> wrote:
>>>>
>>>>>
>>>>> On Aug 10, 2013, at 6:31 PM, Henning Schmiedehausen wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I was toying with the log4j 2 API for a new project and I stumbled
>>>>> over the fact that it uses a generic for Appender<T> without actually
being
>>>>> generic. The only generic part is the Layout. So as a result there is
this
>>>>> weird construct of Appender<SomeSerializableType> which is actually
>>>>> dictated by the layout in use.
>>>>>
>>>>>
>>>>> I'm relatively new to the team, so I don't know much about the
>>>>> reasoning behind making Appender generic, so I can't speak to that. I'm
not
>>>>> personally opposed to removing these generics, but that is a HUGE change.
>>>>>
>>>>> This leads to really interesting constructs such as
>>>>>
>>>>> public abstract class AbstractDatabaseAppender<T extends
>>>>> AbstractDatabaseManager> extends AbstractAppender<LogEvent>
>>>>>
>>>>>
>>>>> Well this is a very different case. The <LogEvent> here is about
>>>>> Layout, just as you said. The <T extends AbstractDatabaseManager>
is
>>>>> completely unrelated to Layout and I am _not_ in favor of removing these
>>>>> generics.
>>>>>
>>>>> I was wondering whether this is necessary as it makes the API very
>>>>> cumbersome to use and read so I removed the generic from Appender and
>>>>> subsequently went through the log4j 2 code base and mostly removed stuff
>>>>> that was no longer needed once that was gone. The result is at
>>>>>
>>>>> https://github.com/apache/logging-log4j2/pull/1
>>>>>
>>>>>
>>>>> The Apache GitHub repository is just a mirror of our SVN repository.
>>>>> We can't accept or use any pull requests there. You need to generate
an SVN
>>>>> patch and attach it to whatever JIRA you create. (As such, you should
close
>>>>> this pull request.)
>>>>>
>>>>> I will also file a JIRA for this.
>>>>>
>>>>> I know that the 2.0 release should be coming soon (being at beta8),
>>>>> but I feel that making that change in the API before it is set in stone
>>>>> with 2.0 woulc be really beneficial for anyone who wants to port code
to
>>>>> 2.0 / write new code.
>>>>>
>>>>>
>>>>> I'm sure there will be plenty of discussion about this over the next
>>>>> few days.
>>>>>
>>>>> Thanks for considering,
>>>>>     Henning
>>>>>
>>>>>
>>>>> Nick
>>>>>
>>>>
>>>>
>>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
View raw message