-
Notifications
You must be signed in to change notification settings - Fork 322
Adding JavaDoc #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding JavaDoc #396
Conversation
| * </table> | ||
| * | ||
| * <p> | ||
| * Complex methods: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word complex has a little bit negative image. We can say this as convenience, utility or helper method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex -> Utility
| /** | ||
| * Writes header of a Binary value. | ||
| * <p> | ||
| * You will call {@link #writePayload(byte[])} or {@link #addPayload(byte[])} method to write body binary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will -> You _MUST_ call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will -> MUST
| * break; | ||
| * case EXTENSION: | ||
| * extension = unpacker.unpackExtensionTypeHeader(); | ||
| * unpacker.readPayload(new byte[extension.getLength()]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readPayload(byte[]) should be modified to return byte[]. Currently it returns void
| * case ARRAY: | ||
| * length = unpacker.unpackArrayHeader(); | ||
| * for (int i = 0; i < length; i++) { | ||
| * readRecursively(unpack); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readRecursively(unpacker)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. unpack -> unpacker
| * case MAP: | ||
| * length = unpacker.unpackMapHeader(); | ||
| * for (int i = 0; i < length; i++) { | ||
| * readRecursively(unpack); // key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unpack -> unpacker
| * </table> | ||
| * | ||
| * <p> | ||
| * To read a byte array, first you call {@link #unpackBinaryHeader} method to get length of the byte array. Then, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if we remove two yous, it still makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/you//g
| * | ||
| * <p> | ||
| * To read an Array type, first you call {@link #unpackArrayHeader()} method to get number of elements. Then, | ||
| * you call unpacker methods for each element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, call ... seems natural
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/you//g
| * | ||
| * @return the next MessageFormat | ||
| * @throws IOException when failed to read the input data. | ||
| * @throws IOException when underlaying input throws IOException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: underlying
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Fixed the typo in all files.
| public void readPayload(byte[] dst) | ||
| throws IOException | ||
| { | ||
| readPayload(dst, 0, dst.length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we return the dst byte array here so that we can use this method like readPayload(new byte[...])?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people will use readPayload(int) for that use case. It's already there. So, I think it's ok to return void here. Having consistency with readPayload(byte[], int off, int len) might be more important to avoid confusion.
| } | ||
|
|
||
| /** | ||
| * Gets size of the written data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size -> the size
| } | ||
|
|
||
| /** | ||
| * Gets copy of the written data as a byte array. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a copy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy -> a copy
| * array-backed buffers. | ||
| * <p> | ||
| * MessageBuffer class itself is optimized for little-endian CPU archtectures so that JVM (HotSpot) can take advantage | ||
| * of the fastest JIT format which skips TypeProfile checking. To ensure this performance, applications must not load |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load -> import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load -> import
| * | ||
| * @param array the byte array that will gack this MessageBuffer | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the -> a new MessageBuffer that wraps the given byte array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| * @param offset The offset of the subarray to be used; must be non-negative and no larger than array.length | ||
| * @param length The length of the subarray to be used; must be non-negative and no larger than array.length - offset | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| * @throws IllegalArgumentException given byte buffer returns false both from hasArray() and isDirect() | ||
| * @throws UnsupportedOperationException given byte buffer is a direct buffer and this platform doesn't support Unsafe API | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
|
||
| /** | ||
| * byte size of the buffer | ||
| * Gets size of the buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the size
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
frsyuki
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied 👍
| * </table> | ||
| * | ||
| * <p> | ||
| * Complex methods: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex -> Utility
| /** | ||
| * Writes header of a Binary value. | ||
| * <p> | ||
| * You will call {@link #writePayload(byte[])} or {@link #addPayload(byte[])} method to write body binary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will -> MUST
| * case ARRAY: | ||
| * length = unpacker.unpackArrayHeader(); | ||
| * for (int i = 0; i < length; i++) { | ||
| * readRecursively(unpack); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. unpack -> unpacker
| * case MAP: | ||
| * length = unpacker.unpackMapHeader(); | ||
| * for (int i = 0; i < length; i++) { | ||
| * readRecursively(unpack); // key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unpack -> unpacker
| * </table> | ||
| * | ||
| * <p> | ||
| * To read a byte array, first you call {@link #unpackBinaryHeader} method to get length of the byte array. Then, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/you//g
| * array-backed buffers. | ||
| * <p> | ||
| * MessageBuffer class itself is optimized for little-endian CPU archtectures so that JVM (HotSpot) can take advantage | ||
| * of the fastest JIT format which skips TypeProfile checking. To ensure this performance, applications must not load |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load -> import
| * | ||
| * @param array the byte array that will gack this MessageBuffer | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| * @param offset The offset of the subarray to be used; must be non-negative and no larger than array.length | ||
| * @param length The length of the subarray to be used; must be non-negative and no larger than array.length - offset | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| * @throws IllegalArgumentException given byte buffer returns false both from hasArray() and isDirect() | ||
| * @throws UnsupportedOperationException given byte buffer is a direct buffer and this platform doesn't support Unsafe API | ||
| * @return | ||
| * @return the new MessageBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
|
||
| /** | ||
| * byte size of the buffer | ||
| * Gets size of the buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
👍 |
This PR adds JavaDoc to common public methods and classes.
I built JavaDoc using sbt's
~doccommand.