mr-smidge commented on a change in pull request #7671:
URL: https://github.com/apache/arrow/pull/7671#discussion_r451216615



##########
File path: csharp/src/Apache.Arrow/Arrays/BinaryArray.cs
##########
@@ -237,7 +329,9 @@ public ReadOnlySpan<byte> GetBytes(int index)
 
             if (IsNull(index))
             {
-                return null;
+                // Note that `return null;` is valid syntax, but would be 
misleading as `null` in the context of a span
+                // is actually returned as an empty span.
+                return ReadOnlySpan<byte>.Empty;

Review comment:
       This clause previously said `return null;`, which was misleading as it 
suggested that the method could be used to identify null values in the array.  
However, `null` in the context of a `ReadOnlySpan<T>` is the same as the empty 
span, and so the method can't distinguish null values from empty ones:
   
   ```csharp
   Assert.True(ReadOnlySpan<byte>.Empty == null)
   ```
   
   The new code is clearer in intent.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to