I beleive we got sidestacked by the question of generics which imo are secondary to the core problem. If we assume that the number and type of each database column is fixed, with some wiggling room allowed for the actual name of the column, once the event type is known the actual structure of the tables is fixed for each DBAppender class. Note that DBAppenderBase already uses generics. Both ch.qos.logback.access.db.DBAppender and ch.qos.logback.classic.db.DBAppender extending ch.qos.logback.core.db.DBAppenderBase. The two DBAppender classes target different table structures. Now if we forget about AccessEvent and the logback-access module and concentrate only on ch.qos.logback.classic.db.DBAppender, letting DBAppender cater for different table structures would make the code too complex and hard to maintain. If the table structure as it exists in DBAppender is deemed too wasteful, we can improve the structure. What I would like to avoid is a DBAppender which is so abstract that each project implements its own version of it. Anyway, DBNameResolver would allow for an indirection of table and column names. It's a small incremental change. If we need to add extra columns for the logger request arguments, we can do that without breaking compatibility too horribly. (Adding columns to a database is usually not a problem). Normalization of various fields makes sense and if it can be achieved without increasing too much the number of queries per insertion, then it would be a good improvement to consider. Instead of debating abstractions, looking at real code would perhaps be more constructive at this stage. On 26.02.2010 16:03, Durchholz, Joachim wrote:
Just a general note:
It think that the number and semantics of columns should be defined in the same class that actually fills them via stmt.setXxx calls. I guess that's what got me thinking about improvement proposals in the first place.
I see three ways to accomplish this: 1. Subclass DBAppender. 2. Use a DBWriter class that logs whatever information is thrown at it. This means generics, because the "whatever information" depends on the kind of appender used but can be statically determined for each appender. (More details on the nature of generics below.) 3. Use a DBNameResolver class that provides column names. The case for generics is the same: different appenders will want to access different but statically predetermined sets of columns.
I wish I had a good and short explanation why generics do not make sense here. As mentioned a little earlier, DBNameResolver provides a service to DBAppender. DBAppender has a DBNameResolver but does not store a collection of DBNameResolvers or process DBNameResolvers.
Making a class generic does not mean that it "contains" or "processes" objects of the parameter class. java.lang.Comparable is the classical counterexample to such interpretations of genericity. Generics are a means of consistently varying a type across result and parameter positions within a class or class sub-hierarchy. This can be used to make containers, but there are many more use cases than that.
Besides that, generics in Java are quite clumsy to use.
Agreed. Code completion does a lot to alleviate the pain, and getting used to the way that generics look like does help, too.
More to the point and as you probably know already, generics get erased (type erasure) as explained in
http://java.sun.com/docs/books/tutorial/java/generics/erasure.html
That does not affect the case at hand. As long as the code gets run through a Java compiler, you will still get static guarantees that your code does not violate the type constraint. The JVM will erase the type parameter and hence cannot enforce the constraints at class load time, plus type erasure will prevent some type checks at the Java level, but this is not the case for the code I proposed. (Also, type erasure is schedules for removal. I hope it will go away with Java 7.)
One cannot do much with java generics really.
My personal experience has been otherwise. I routinely write generics to good effect.
There is a learning curve. I worked from an Eiffel background, which is a bit less refined about type bounds than Java, and it took me around a week to wrap my mind about the basic workings and another week to pick up some fine points. I reaped benefits, too. I have converted some of my code to generics, and I found several cases where type casts and string discriminators that I had considered statically safe were, in face, unsafe and just waiting for the rare occasion to throw an exception. Equivalently, I got rid of a lot of "else { throw new RuntimeException ("This can't happen"); }" type of code.
YMMV :)
----
Anyway, what's your second idea?
I'd simply use overridable functions in DBAppender to provide the SQL for database access.
So:
class DBAppender { protected String eventTableName() { return "event_log"; } protected String eventTimestampColumnName () { return "timestmp"; } ... protected String insertPropertiesSql; protected void subAppend(...) { if (insertPropertiesSql == null) { insertPropertiesSql = "INSERT INTO " + eventTableName() + "(" + eventTimestampColumnName() + ", " ... } ... // existing code goes here } }
Subclasses can easily override the table and column names to their taste. (Sounds simpler to me than any of the alternatives. DBAppender should not compute anything, it should be a thin layer that just spits out the data that it's given. It shouldn't even be merging properties, it should be given pre-merged property sets by a caller... or an object that does the merge on request. Hey, LoggingEvent could do that, all input for mergePropertyMaps is from a LoggingEvent anyway.) (Oops. That went off on a tangent. Sorry for that.)
Regards, Jo _______________________________________________ logback-dev mailing list logback-dev@qos.ch http://qos.ch/mailman/listinfo/logback-dev