tustvold commented on code in PR #2711:
URL: https://github.com/apache/arrow-rs/pull/2711#discussion_r968713644


##########
arrow-schema/src/schema.rs:
##########
@@ -205,13 +211,13 @@ impl Schema {
     }
 
     /// Find the index of the column with the given name.
-    pub fn index_of(&self, name: &str) -> Result<usize> {
+    pub fn index_of(&self, name: &str) -> Result<usize, ArrowSchemaError> {
         (0..self.fields.len())
             .find(|idx| self.fields[*idx].name() == name)
             .ok_or_else(|| {
                 let valid_fields: Vec<String> =
                     self.fields.iter().map(|f| f.name().clone()).collect();
-                ArrowError::InvalidArgumentError(format!(
+                ArrowSchemaError::Field(format!(

Review Comment:
   I have to confess I personally am not a massive fan of highly structured 
error reporting, it has a tendency to be hard to maintain, and impractical to 
actually use.
   
   My personal preference is for high-level error categories, to allow for 
custom programatic handling in the caller, but with opaque payloads. 
   
   This allows:
   
   * Clients can easily handle classes of error instead of a large list of 
variants
   * Additional error context can be added without this being a breaking change
   * The error handling logic is kept relatively concise
   
   Imo the value of error enumerations is if the client wishes to do something 
special in response to them, if they're just going to format it to a string, 
the variants are just code and API bloat. I appreciate opinions may differ on 
that though 😅



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