Owned [NamedField] for Fields

[sunng87]

The current signature of Fields::Named hold a borrowed slice of NamedField. As far as I can tell, it's designed for 'static definition of rust structs. This makes it difficult to define dynamic Structable for a map like we talked in #53 .

Perhaps we can make it a Cow for the dynamic scenario.

[hawkw]

@sunng87 just to clarify here, is it the slice that should be owned, or the NamedFields that should be owned?

[sunng87]

@hawkw I think I need for both. I'm trying to construct a Structable from a JSON object, which may have dynamic fields that are determined at runtime, in order to get O(1) access to the fields via valuable API.

Type like this works best for my scenario:

pub enum Fields<'a> {
    /// Named fields
    Named(&'a [NamedField<'a>]),

    OwnedNamed(Vec<OwnedNamedField>),  // where OwnedNamedField hold an owned String instead of str

    /// Unnamed (positional) fields or unit
    Unnamed,
}

[hawkw]

Right, I'm looking into a possible change like this. But, I'm not totally sure if I understand why this change is necessary. If you have a Vec of field names, can't you just pass a borrowed slice of that Vec into Fields::Named?

[sunng87]

That sounds reasonable. It has been a while since last time I'm running into this issue. Let me check my feature branch again to refresh my context about this.

[hawkw]

@sunng87 let me know if that solution works out for you; I'd like to get this issue figured out soon, because if we do add code to make owned named fields possible, we would probably have to make some breaking changes, and it would be nice to figure out what's necessary here before releasing 0.1.

[sunng87]

Sorry for late response. I may still have issue with current NamedField vector. My use-case is to implement Structable for serde_json::Value and its internal Map<String, Value>. Because the map has dynamic fields so I use StructDef::Dynamic here.

impl<'a> Structable for Map<String, Json> {
    fn definition(&'a self) -> StructDef<'a> {
        let field_defs: Vec<NamedField<'a>> = Vec::new();
        // add field names

        let fields = Fields::Named(&field_defs);

        let def = StructDef::Dynamic {
            name: "Json",
            fields,
        };

        def
    }
}

There are two issues from this piece of code:

1. The lifetime 'a from StructDef<'a> should be same as the Map because the field names in StructDef borrows str from the map. But I got error[E0308]: method not compatible with trait with my code above.
2. The NamedField vector is created within the function so it's impossible to borrow it outside.

OwnedNamed(Vec<OwnedNamedField>) in my previous comment can fix both 1 and 2 issue.

[hawkw]

Hmm, I see why you can't borrow the fields here, so you're right that an owned variant might be necessary. Thanks for the example!

[sunng87]

Another suggestion is to make name of StructDef::Dynamic an Option<String> because the json object is typically anonymous in term of a type.

[carllerche]

At some point, I'd say that the type isn't a structable but a mappable.

I think it would be fair to add some additional trait fns that make it easier to work w/ json objects. Eg. get(&str) -> Option<Value<'_>> or something like that.

[sunng87]

The only concern with Mappable is it doesn't seem to have O(1) access to value with given key name.