clairemcginty commented on PR #31025:
URL: https://github.com/apache/beam/pull/31025#issuecomment-2125259557
> Thank you for contributing this! The following three proposals go together
and made more sense to provide in one comment, rather than separately. May we
consider instead of the new custom class:
>
> 1. two private static methods as shown below:
>
> ```java
> private static <T> T elementOf(Coder<T> coder, byte[] bytes) throws
CoderException {
> if (coder instanceof ByteArrayCoder) {
> return (T) bytes;
> }
> return CoderUtils.decodeFromByteArray(coder, bytes);
> }
>
> private static <T> byte[] bytesOf(Coder<T> coder, T element) throws
CoderException {
> if (element instanceof byte[]) {
> return (byte[]) element;
> }
> return CoderUtils.encodeToByteArray(coder, element);
> }
> ```
>
> 2. Refactoring the SortValuesDoFn's processElement method with:
>
> ```java
> for (KV<SecondaryKeyT, ValueT> record : records) {
> sorter.add(KV.of(
> bytesOf(keyCoder, record.getKey()),
> bytesOf(valueCoder, record.getValue()))
> );
> }
> ```
>
> 3. Refactoring DecodingIterator's next with:
>
> ```java
> SecondaryKeyT secondaryKey = elementOf(keyCoder, next.getKey());
> ValueT value = elementOf(valueCoder, next.getValue());
> return KV.of(secondaryKey, value);
> ```
Thanks for the review @damondouglas ! Sorry for the late reply, I've been
out of office, but will apply these suggestions today or tomorrow 👍
--
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]
