Wrong contract of translateTo() method in EventTranslator interface

[GoogleCodeExporter]

I think there is a big bug in EventTranslator interface. EventTranslator define 
a T translateTo(T event, long sequence) method. So one hopes that translateTo() 
method returns something.
After looking at EventPublisher object source, one can read :

{{{
private void translateAndPublish(final EventTranslator<E> translator, final 
long sequence)
{
   try
   {
       translator.translateTo(ringBuffer.get(sequence), sequence);
   }
   finally
   {
       ringBuffer.publish(sequence);
   }
}
}}}

*So method translateTo() is called but never wait for a return.*

You can even return null in the translateTo() method, no problem ! :

_(EventWrapper is a simple POJO wrapping a T event)_
{{{
public EventWrapper translateTo(final EventWrapper event, final long sequence) {
    event.setEvent(wrappedEvent);
    return null;
}
}}}

Message to disruptor developers You may define the translateTo() method in the 
EventTranslator interface as
{{{
<T> void translateTo(T event, long sequence)
}}}

Original issue reported on code.google.com by cmarco...@gmail.com on 19 May 2012 at 1:16

[GoogleCodeExporter]

Sorry for {{{ and }}}, I thought it was the same as wiki syntax.

Original comment by cmarco...@gmail.com on 19 May 2012 at 1:17

[GoogleCodeExporter]

Did a mistake, should have written 
void translateTo(T event, long sequence)

Original comment by cmarco...@gmail.com on 19 May 2012 at 2:11

[GoogleCodeExporter]

You're correct.  The translateTo method should have a void return type.  I'll 
change that for the next release.

Original comment by mike...@gmail.com on 20 May 2012 at 8:52

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I think lmax team could take it into consider that why not trying to add some 
helpful way to has the real return event for most cases for request-response 
are much needed,as 
we need to fetch the result from the event and also the event itself can hold 
the phase of the result resort to the event processor.

Original comment by jerrysco...@gmail.com on 20 May 2012 at 1:44

[GoogleCodeExporter]

As I know that some third party have tried for that mentioned above to add some 
similar way like EventResultTranslator to add support for that.

Original comment by jerrysco...@gmail.com on 20 May 2012 at 1:45

[GoogleCodeExporter]

It think this is historical and no longer needed.  The event use to be abstract 
and had a commit method on it back in the early days of the Disruptor.  Since 
events no longer need to extend the abstract implementation the commit method 
was removed and thus does not need to be returned.

Original comment by mjpt...@gmail.com on 24 May 2012 at 7:21

[GoogleCodeExporter]

Original comment by mike...@gmail.com on 24 May 2012 at 6:29

• Changed state: Fixed

[GoogleCodeExporter]

Hi :
      I think I did not clearly explain what I had wanted to express for.

      One side,that's probably not worthy of adding any return param
towards translateTo() method as the issue has pointed out. for another,
as EventTranslator is for generic use bound to the params as we used during
the processing between producer and consumer, we sometimes want to fetch
such Event resort to EventFactory as Event could hold
the processing result with the EventProcessors. so I suggest we could add
some getCurrentEvent method to load such result.  Here comes my example to
reach that purpose (but I was not quite sure about it):

public final class EventResultPublisher<E> {
private final static transient Logger logger =
LoggerFactory.getLogger(EventResultPublisher.class);

private final AtomicLong waitAtSequence = new AtomicLong();

private final RingBuffer<E> ringBuffer;

private final EventProcessor[] eventProcessors;

public EventResultPublisher(final RingBuffer<E> ringBuffer,
EventProcessor[] eventProcessors) {
this.ringBuffer = ringBuffer;
this.eventProcessors = eventProcessors;
}

public void publishEvent(final EventTranslator<E> translator) {
translateAndPublish(translator, this.ringBuffer.next());
}

private void translateAndPublish(final EventTranslator<E> translator, final
long sequence) {
try {
this.waitAtSequence.set(sequence);
translator.translateTo(this.ringBuffer.get(sequence), sequence);
} finally {
this.ringBuffer.publish(sequence);
}
}

public E getCurrentEvent(long timeoutforeturnResult) {
SequenceBarrier barrier =
this.ringBuffer.newBarrier(Util.getSequencesFor(this.eventProcessors));

try {
long avaliable = barrier.waitFor(this.waitAtSequence.get(),
timeoutforeturnResult, TimeUnit.MILLISECONDS);

return this.ringBuffer.get(avaliable);
} catch (Exception ex) {
logger.error("fetch the return fail, timeout params is {" +
timeoutforeturnResult + "},ԭ��{" + ex.getMessage() + "}");
}

return null;
}

public E getCurrentEvent() {
SequenceBarrier barrier =
this.ringBuffer.newBarrier(Util.getSequencesFor(this.eventProcessors));

try {
long avaliable = barrier.waitFor(this.waitAtSequence.get());

return this.ringBuffer.get(avaliable);
} catch (Exception ex) {
logger.error("fetch the result fail, reason{" + ex.getMessage() + "}");
}

return null;
}

}

        Hope disruptor team could help me to feeback about it whether it's
right or all right or some trial mistake exits.

        Best Wishes!

Original comment by jerrysco...@gmail.com on 28 May 2012 at 2:02