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
[email protected]
http://qos.ch/mailman/listinfo/logback-dev
_______________________________________________
logback-dev mailing list
[email protected]
http://qos.ch/mailman/listinfo/logback-dev
