On Jan 10, 2005, at 12:33 PM, Ceki Gülcü wrote:
At 05:43 PM 1/10/2005, Curt Arnold wrote:
I said Object.hashCode, the underlying implementation if you had called super.hashCode() within logging event. It has similar characteristics to a memory address,
Yes, the Object.hashCode implementation is computed over the object's addres. However, two equal objects *must* return the same hashcode. In particular, two successive deserializations of the same event *must* return the same hashcode. Object.hashCode does not respect this fundamental constraint.
...
Isn't this a self-defeating argument? If Object.hashCode can be used to check for equality, you might as well implement the equals() method as:
class LoggingEvent {
public boolean equals(Object r) { return this == r; } }
What I was suggesting as a potential approach (with the drawbacks mentioned), is the value of hashCode at the time of creation could be obtained and stored as a distinct member that would be be used to distinguish objects that are identical in value in all other respects. That is replacing:
LoggingEvent() {
...
synchronized(LoggingEvent.class) {
sequenceNumber = sequenceCount++;
}
}with
LoggingEvent() {
...
eventID = super.hashCode();
}Two logging events that are identical in all other respects would have a high likelihood of having distinct values for eventID and acquiring the hash code would not incur an synchronization lock.
I'd love to hear your thoughts behind adding the setMessage and other mutators to LoggingEvent. It is generally a very good thing for objects to be immutable since you can eliminate a score of potential synchronization problems. In log4j 1.3, LoggingEvent's have become mutable which means that we have to think about scenarios like a LoggingEvent gets placed in an AsynchAppender then gets modified in a filter. The AsyncAppender may or may not see the new message. It is usually much easier to prevent synchronization issues by designing immutable objects than to ensure that mutable but non synchronized classes are used correctly.
In 1.3, LoggingEvents are still immutable. Just have a look at its setter methods. The only mutable part is the LoggingEvent 'properties' member field. (The sequence number is mutable but that's just an omission. I'll correct it shortly.)
I missed that most of the setters are protected by checks that they haven't been set previously. setSequenceNumber and setTimestamp don't have those protections. setRenderedMessage looks like it could be used to override the creation message if the original message hadn't been rendered.
I did an experiment by changing all the mutator classes to private access. The only thing that broke was PatternParserTest which used:
event = new LoggingEvent();
event.setLogger(logger);
event.setTimeStamp(now);
event.setLevel(Level.INFO);
event.setMessage("msg 1");to construct an LoggingEvent, changing it to:
event = new LoggingEvent(logger.getClass().toString(), logger, now, Level.INFO, "msg 1", null);
compiled and all tests passed.
From that usage and your comments, it appears that your intent was not to allow filters to modify messages (which was my mistaken guess at the justification), but to provide an incremental "construction" mechanism instead of passing all the arguments in the constructor.
A much better approach to this would be to add a LoggingEventBuilder (or LoggingEventFactory) class that would serve the same role as StringBuffer does for String. The code in PatternParserTest would look like:
LoggingEventBuilder builder = new LoggingEventBuilder();
builder.setLogger(logger);
builder.setTimeStamp(now);
builder.setLevel(Level.INFO);
builder.setMessage("msg 1");
LoggingEvent event = builder.newInstance();Advantages of the approach:
Violations of invariants (like an LoggingEvent must have a level) can raise exceptions in the newInstance call. The other approach would allow you to pass a LoggingEvent without a level which would likely cause problems in the processing.
Multiple objects that are slightly different can be created by repeated invocations of newInstance.
Member variables can be marked as final.
I assume that part of you motivation is to simplify the calling signatures of the constructors. Instead of a zero-parameter constructor, this pattern could be implemented with a one-parameter constructor.
public class LoggingEvent {
public LoggingEvent(final LoggingEventBuilder builder) {
...
}
...
}public class LoggingEventBuilder {
public LoggingEvent newInstance() {
return new LoggingEvent(builder);
}
}This technique allows new fields to be added to the LoggingEvent without changing the signature of the LoggingEvent constructor.
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
