My intention was to make the code more understandable for readers. Because if 
we do explicitly casting and leave less guess room for readers, readers will 
know the same type of values are compared, and they do not need to question the 
correctness because of implicitly casting somewhere. At least, readers do not 
need to search  how `assertEquals` is implemented for different types. From 
this perspective, `assertEquals(Byte.valueof((byte) 1), 
nestedRow.getByte("aByte"))` is ugly, but necessary.

After I read the code, seems like the family of `assertEquals` in JUnit is 
simpler. `assertEquals(Byte.valueof((byte) 1), nestedRow.getByte("aByte"))` 
will be converted to `assertEquals(object, object)`, and 
`object.equals(object)` will be called. In `Byte` implementation, as Anton 
pointed out, `Byte.equals()` accepts `Object` and does a casting anyway. So 
from implementation perspective, there is no difference among approaches in 
this discussion.

Since there isn't a perfect way to make the code clear, I am ok with either the 
original way in this PR, or other discussed way to implement it.

[ Full content available at: https://github.com/apache/beam/pull/6383 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to