stankiewicz commented on code in PR #37963:
URL: https://github.com/apache/beam/pull/37963#discussion_r3101619462
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -37,7 +37,9 @@
import org.checkerframework.checker.nullness.qual.Nullable;
/**
- * A {@link Coder Coder<T>} defines how to encode and decode values of
type {@code T} into
+ * Use {@link CoderProperties} to verify the correctness of a {@link Coder}
implementation.
Review Comment:
does have to be first sentence?
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
Review Comment:
there a lot of duplicate javadoc, please simplify it a bit.
##########
.gitignore:
##########
Review Comment:
shouldn't be part of commit
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -60,6 +74,14 @@ public abstract class Coder<T> implements Serializable {
* @deprecated To implement a coder, do not use any {@link Context}. Just
implement only those
* abstract methods which do not accept a {@link Context} and leave the
default
* implementations for methods accepting a {@link Context}.
+ * <p>There are two common contexts:
Review Comment:
why "common"?
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -52,6 +54,18 @@
* <p>All methods of a {@link Coder} are required to be thread safe.
*
* @param <T> the type of values being encoded and decoded
+ *
+ * <p>When implementing a {@link Coder}, it is important to understand the role
Review Comment:
Does it have to be part of this javadoc?
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -117,8 +139,29 @@ public String toString() {
* know how many bytes to read when decoding. A common approach is to prefix
the encoding with the
* element's encoded length.
*
+ * <p>The encoding behavior depends on the {@link Context}:
+ *
+ * <ul>
+ * <li>In {@link Context#OUTER}, the value occupies the remainder of the
stream, so no
Review Comment:
those descriptions are inconsistent.
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -117,8 +139,29 @@ public String toString() {
* know how many bytes to read when decoding. A common approach is to prefix
the encoding with the
* element's encoded length.
*
+ * <p>The encoding behavior depends on the {@link Context}:
+ *
+ * <ul>
+ * <li>In {@link Context#OUTER}, the value occupies the remainder of the
stream, so no
+ * additional length information is required.
+ * <li>In {@link Context#NESTED}, the value is part of a larger structure,
so the encoding must
+ * include sufficient information (such as length prefixes) to allow
correct decoding of
+ * subsequent values.
+ * </ul>
+ *
+ * <p>Implementations must ensure that encoded values are self-delimiting
when used in a nested
+ * context.
+ *
* @throws IOException if writing to the {@code OutputStream} fails for some
reason
* @throws CoderException if the value could not be encoded for some reason
+ *
+ * <p>Implementers must ensure that encoding in {@link Context#NESTED}
Review Comment:
this is duplicate but written slightly differently.
##########
sdks/java/core/src/main/java/org/apache/beam/sdk/coders/Coder.java:
##########
@@ -52,6 +54,18 @@
* <p>All methods of a {@link Coder} are required to be thread safe.
*
* @param <T> the type of values being encoded and decoded
+ *
+ * <p>When implementing a {@link Coder}, it is important to understand the role
+ * of the encoding {@link Context}. The context determines whether the encoded
+ * value must be self-delimiting or can consume the remainder of the stream.
+ *
+ * <p>Implementations should ensure that values encoded in {@link
Context#NESTED}
+ * include sufficient length information (such as prefixes) to allow correct
+ * decoding of subsequent values.
+ *
+ * <p>Use {@link CoderProperties} in tests to verify correctness, determinism,
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
