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:
[email protected]
