Re: LoggingEvent.sequenceCount

[Curt Arnold]

On Jan 10, 2005, at 3:05 PM, Ceki G�lc� wrote:

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.

So, "all" you are suggesting is to replace
class LoggingEvent {
  long sequenceNumber;
  ...
  LoggingEvent(...) {
    ...
     sequenceNumber = sequenceCount++;
  }
}
with
class LoggingEvent {
  int eventID ;
  ...
  LoggingEvent(...) {
    ...
     eventID = super.hashCode();
  }
eventID replacing sequenceNumber. The existing implementations of 
equals() and hashCode() would remain the same. Correct?

In the original message, I outlined two approaches and their perceived 
drawbacks.  One was just replacing sequenceCount with the value of 
super.hashCode.  This would likely work okay, but the short-lifetime of 
LoggingEvent's might result in recurring values for hashCode and that 
would have to be investigated.

My favored approach in that message was a full value comparison 
implementation of equals and hashCode.  I had not expected the 
requirement that identical but distinct logging messages not compare as 
equal.  To address that short-coming, you could still add the eventID 
and use that in a full value comparison implementation of equals.

It may be adequate for equals and hashCode to only check the timestamp, 
event id and message text.  I think it would be very very rare that two 
messages in the wild would have duplicate values for all three, and if 
they did having Chainsaw drop one as a duplicate would not be a 
significant issue.
EventDetailLayout.java  
log4j-1.3/src/java/org/apache/log4j/chainsaw/layout line 196
LayoutEditorPane.java   
log4j-1.3/src/java/org/apache/log4j/chainsaw/layout line 186
DBReceiverJob.java      log4j-1.3/src/java/org/apache/log4j/db  line 98
LogFilePatternReceiver.java     
log4j-1.3/src/java/org/apache/log4j/varia
UtilLoggingXMLDecoder.java      
log4j-1.3/src/java/org/apache/log4j/xml line 380
XMLDecoder.java                 
log4j-1.3/src/java/org/apache/log4j/xml line 386
PatternParserTest.java  
log4j-1.3/tests/src/java/org/apache/log4j/pattern line 57

Apparently you did a "clean" before building.  For some reason, those 
files didn't get recompiled when I did my ultra quick test and only 
PatternParserTest got recompiled.  They do all seem to be trying to 
build a new LoggingEvent instead of modifying an existing one.



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.
That is correct.
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.
Good point.
Multiple objects that are slightly different can be created by 
repeated invocations of newInstance.
Yes? How so?
LoggingEventBuilder builder = new LoggingEventBuilder();
builder.setLogger(logger);
builder.setTimeStamp(now);
builder.setLevel(Level.INFO);
builder.setMessage("msg 1");
LoggingEvent event = builder.newInstance();
builder.setMessage("msg 2");
LoggingEvent event2 = builder.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.
Sounds good.

I'll take a stab at just the Builder rework.  Do you have any 
objections to my committing it if it seems pretty straightforward and 
passes all the tests?

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]