siddharthaDevineni commented on code in PR #21601:
URL: https://github.com/apache/kafka/pull/21601#discussion_r2892481257
##########
clients/src/main/java/org/apache/kafka/common/utils/Bytes.java:
##########
@@ -34,6 +43,14 @@ public class Bytes implements Comparable<Bytes> {
// cache the hash code for the string, default to 0
private int hashCode;
+ /**
+ * Creates a Bytes instance wrapping the given byte array.
+ *
+ * <p>The provided array becomes the backing storage for the object.
+ *
+ * @param bytes the byte array to wrap, or null
+ * @return a new Bytes instance, or null if the input is null
+ */
public static Bytes wrap(byte[] bytes) {
if (bytes == null)
Review Comment:
Thank you Chia, you are right on the inconsistency of null handling.
Actually, `toString()` does handle null - the private `toString(byte[], int,
int)` method has a null check:
```java
private static String toString(final byte[] b, int off, int len) {
if (b == null)
return result.toString(); // Returns empty string
```
But the method `compareTo()` will throw an NPE if the internal `bytes` array
is null, since the comparator accesses `bytes.length` without any null check.
I have added a null check to the constructor using
`Objects.requireNonNull()` so that:
- `wrap(null)` continues to return null (backward compatible)
- `new Bytes(null)` throws an NPE immediately (fail fast)
- All Bytes instances are guaranteed to have a non-null bytes array
- No null checks needed in methods like `compareTo()`
This makes it now consistent i guess?
--
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]
