Changes serialize() to take an OutputStream as a parameter. #69
Conversation
| * @return the given {@link OutputStream} | ||
| */ | ||
| public abstract ByteBuffer serialize(); | ||
| public abstract OutputStream serialize(OutputStream output); |
There was a problem hiding this comment.
I have it return the output stream in case someone wants to chain calls together.
I am thinking of adding a 'throws IOException' in case writing to the stream has a problem.
There was a problem hiding this comment.
I have it return the output stream in case someone wants to chain calls together.
IMO a void return type makes it clearer that the method writes to the input, and that the user doesn't need to look at the return value.
| .newBuilder().setTags(builder.toString()).build().toByteArray()); | ||
| try { | ||
| output.write(StatsContextProto.StatsContext | ||
| .newBuilder().setTags(builder.toString()).build().toByteArray()); |
There was a problem hiding this comment.
This can call StatsContextProto.StatsContext.writeTo(OutputStream) now.
sebright2
left a comment
sebright2
left a comment
There was a problem hiding this comment.
Should we keep the old serialization methods, too?
|
I don't think there's any point to - we don't need them for backward compatibility so we're probably better off getting rid of them entirely. |
2 participants
If we decide to go with this change, there will be a dual change to deserialize to take an InputStream rather than a ByteBuffer.