Would this not solve that problem?
public interface Layout {
byte[] giveMeAByteArray(LogEvent);
String giveMeAString(LogEvent);
LogEvent giveMeACopy(LogEvent); // LogEvent implements Serializable
...
}
On Wed, Aug 14, 2013 at 11:50 PM, Gary Gregory <[email protected]>wrote:
> On Wed, Aug 14, 2013 at 10:45 AM, Remko Popma <[email protected]>wrote:
>
>> Does toSerializable need to be a single method?
>> Can't we have separate methods that return different things?
>> One method that returns a String, and a different method that returns a
>> LogEvent copy?
>>
>
> See Ralph's message on this thread dated Sun, Aug 11, 2013 at 3:47 PM. I
> think that addresses the issue. I am guessing the 3:47 PM is in my
> timezone, UTC-05.
>
> Gary
>
> That would simplify things a lot, I think.
>>
>>
>> On Wed, Aug 14, 2013 at 11:37 PM, 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
>>>
>>
>>
>
>
> --
> 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
>
