BlakeOrth opened a new issue, #9113:
URL: https://github.com/apache/arrow-rs/issues/9113

   **Is your feature request related to a problem or challenge? Please describe 
what you are trying to do.**
   
   
   When using the 
[`VartiantBuilder`](https://docs.rs/parquet/latest/parquet/variant/struct.VariantBuilder.html#example-create-a-variantlist-an-array)
 I found it felt a bit clunky that the `finish()` method returned a raw tuple 
of  `(Vec<u8>, Vec<u8>)`. Given the 0-copy nature of the `Variant` type, I 
understand why this is the case, but I think the ergonomics of this could be 
improved, which would help the code be more self-documenting and in turn 
hopefully more clear for users.
   
   **Describe the solution you'd like**
   I don't feel like this needs a major overhaul, and I'm pretty sure some 
simple type aliasing might improve this. 
   
   (pseudo-code)
   
   ```rust
   // There are probably better names here, but user usage with 
Variant::Metadata and Variant::ValueData seems
   // relatively nice
   pub type Metadata = Vec<u8>;
   pub type ValueData = Vec<u8>;
   pub type Data = (Metadata, ValueData);
   ```
   
   Then, we could have something like:
   ```rust
   // VariantBuilder
   pub fn finish(&self) -> Data {
   ...
   }
   
   // Variant
   
    // blissfully ignoring lifetime specifiers for brevity
   pub fn try_new(data: &Data) -> Result<Self, ArrowError> {
       Variant::try_new_from_parts(&data.0, &data.1)
   }
   
   // The existing `new()` methods could be kept, but renamed to maintain 
functionality
   // and use the familiar "from_parts" convention
   pub fn try_new_from_parts(metadata: &'m [u8], value: &'v [u8]) -> 
Result<Self, ArrowError> {
   ...
   }
   ```
   
   Usage on the user's end would then look more like:
   ```rust
   let mut builder = VariantBuilder::new();
   builder.append_value(12);
   let data = builder.finish();
   
   let variant = Variant::new(&data);
   
   // or from parts
   let (meta, value) = (data.0, data.1) // this admittedly doesn't read quite 
as nicely
   let variant2 = Variant::new_from_parts(&meta, &value);
   ```
   
   **Describe alternatives you've considered**
   An alternative is, of course, doing nothing. There's clearly nothing 
explicitly incorrect about the existing implementation perhaps aside from it 
being easy to forget which element of the `(Vec<u8>, Vec<u8>)` is value data 
and which is metadata.
   
   **Additional Context**
   If this seems like a direction people would like to move I would be happy to 
contribute via PR or PR review etc.
   
   


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