On March 13, 2015, 11:43 p.m., Yasuhiro Matsuda wrote: > > The inheritance of MessageWriter from BufferingOutputStream is a bit > > confusing, since it will always use itself in the writePayload function > > parameter. > > > > I feel it is more clear to read the code if we just let MessageWriter > > contains a var of BufferingOutputStream; and instead of pass in the > > function logic of writing the message, we can just pass in messages and > > offsetCounter in the write() call which will then write the messages itself. > > Yasuhiro Matsuda wrote: > It is true that the current code writes only through writePayload. But I > wanted MessageWriter to be a subclass of OutputStream to be more generic in > case we need to write additional inforation other than messages in future. > > Guozhang Wang wrote: > As for now MessageWriter's only public function is write(key, codec) > (valueWritefunction), which is used for writing a single message. Also its > private functions withCrc32Prefix / withLengthPrefix is only used for message > writing. So it is a bit unclear about your motivation in future extensions. > Could you elaborate a bit more on that?
I don't know future usages at this point. Besides, withCrc32Prefix uses internal structure of BufferingOutputStream for efficiency. Does this justify the inheritance? If we don't do so, the code will be more cluttered. - Yasuhiro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/#review76454 ----------------------------------------------------------- On March 16, 2015, 10:19 p.m., Yasuhiro Matsuda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31742/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 10:19 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-527 > https://issues.apache.org/jira/browse/KAFKA-527 > > > Repository: kafka > > > Description > ------- > > less byte copies > > > Diffs > ----- > > core/src/main/scala/kafka/message/ByteBufferMessageSet.scala > 9c694719dc9b515fb3c3ae96435a87b334044272 > core/src/main/scala/kafka/message/MessageWriter.scala PRE-CREATION > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/31742/diff/ > > > Testing > ------- > > > Thanks, > > Yasuhiro Matsuda > >
