Gary, I do not see your point here.
In your example, SerializedLayout would have a concrete type for the
toSerializable(LogEvent) method and XMLLayout would have another.
So
new SerializedLayout().toSerializable(logEvent) returns a LogEvent
XMLLayout.createLayout().toSerializable(logEvent) returns a String
You only lose the concrete type when you to not have the actual Layout
instance but e.g. what appender.getLayout() returns (a Layout<? extends
Serializable> in which case all you can assume is that
toSerializable(LogEvent) returns something serializable.
I honestly do not see where you need your type cast.
Thanks,
Henning
On Wed, Aug 14, 2013 at 7:37 AM, Gary Gregory <[email protected]>wrote:
> On Wed, Aug 14, 2013 at 9:52 AM, Remko Popma <[email protected]>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 <[email protected]>wrote:
>>
>>> On Wed, Aug 14, 2013 at 4:32 AM, Remko Popma <[email protected]>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 <
>>>>> [email protected]> 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: [email protected] | [email protected]
>>> 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: [email protected] | [email protected]
> 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
>
