I wouldn't mind taking another stab at this. Currently in transit, cannot check source, But it seems to me that if we know we eventually need a byte array, then why not have that as part of the contract of the Layout interface. Do we ever serialize into anything else than a byte array?
If Layout just returned a fixed byte array (or Serializable object, but why not just the byte array...), then we would not need the generics anymore. AbstractStringLayout can be implemented with an abstract method, say, public String serializeToString(LogEvent); for example, that subclasses need to implement. I'll take a more detailed look when I get back later today. On Monday, August 12, 2013, Ralph Goers wrote: > I think I want to comment on the generic appenders a bit more. > > In Log4j 1.x Layouts always return Strings. That is actually very > limiting. Logback started that way but then switched from Layouts to > Encoders. As described at http://logback.qos.ch/manual/encoders.html, > Encoders transform the LogEvent into a byte array and also write to an > OutputStream. In Log4j 2 the Appenders (which usually delegate to a > Manager) are responsible for managing and writing to the OutputStream. This > means Layouts should return byte arrays, not Strings, to provide the > flexibility that is required. However a large number of Layouts to return > Strings. All of these will extend AbstractStringLayout, which will handle > converting from the String to a byte array. Since AbstractStringLayout > cannot guarantee that its subclasses return a String without the generic it > would have to do: > > Serializable serializable = toSerializable(event); > if (serializable instanceof String) { > ((String) serializable).getBytes(charset); > } else { > throw new IllegalArgumentException("Some error message about bad type"); > } > > instead of just > > toSerializable(event).getBytes(charset); > > Turning what is now a compile time error into a runtime error. > > My recollection was that Gary tried this once and found that removing the > generic had issues in other places but I don't recall what they were. > > Ralph > > > > On Aug 11, 2013, at 12:19 PM, Ralph Goers wrote: > > Gary and I have discussed the generic on the Appender before. It is > problematic to have it and just as problematic to leave it off. IOW, it is > awkward either way. > > As Nick, points out the "interesting construct" below has absolutely > nothing to do with the generic on the Appender. > > I don't know how to compare your pull request to trunk since that is > maintained in svn, at least for now. > > Ralph > > > On Aug 10, 2013, at 4: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. > > This leads to really interesting constructs such as > > public abstract class AbstractDatabaseAppender<T extends > AbstractDatabaseManager> extends AbstractAppender<LogEvent> > > 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 > > 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. > > Thanks for considering, > Henning > > > >
