emilk commented on code in PR #10759:
URL: https://github.com/apache/arrow-rs/pull/10759#discussion_r3842460937


##########
arrow-array/src/array/byte_array.rs:
##########
@@ -296,8 +296,10 @@ impl<T: ByteArrayType> GenericByteArray<T> {
     /// Returns true if all data within this array is ASCII
     pub fn is_ascii(&self) -> bool {
         let offsets = self.value_offsets();
-        let start = offsets.first().unwrap();
-        let end = offsets.last().unwrap();
+        // An `OffsetBuffer` is never empty, but an empty array is ASCII 
either way
+        let (Some(start), Some(end)) = (offsets.first(), offsets.last()) else {
+            return true;
+        };

Review Comment:
   I think this is just defensive coding. It replaces one type of dead code 
(`unwrap`) with another (`return true`).
   
   If indeed all constructors of `OffsetBuffer` ensures it is non-empty, then a 
better solution is to add `first()/last()` accessors on it that returns values 
rather than `Option`s. I'll take a look.



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

Reply via email to