aerospike / aerospike-client-rust

Rust client for the Aerospike database
https://www.aerospike.com/
Other
82 stars 26 forks source link

More to_type functions for Value #99

Closed jonas32 closed 1 year ago

jonas32 commented 3 years ago

Value currently only has a to_string method. As far as i understand thats more meant as some sort of "debug" function to just print whats inside the value. I guess it might be a good idea to add more functions for other Data Types. This functions could return a Result with the Value or an Error if the Type is not correct. Currently the only way to do this is pattern matching. But if you need to match for example 10 bins, that becomes very ugly. The type of the bins is probably known in most applications anyways. A strictly typed language like rust more or less forces you to know when you write it. Im thinking about something like

pub fn to_map(&self) -> Result<HashMap<Value, Value>, TypeError> {
    match &self {
        Value::HashMap(map) => Ok(map)
        _ => Err(TypeError::new())
    }
}

pub fn to_list(&self) -> Result<Vec<Value>, TypeError> {
    match &self {
        Value::List(list) => Ok(list)
        _ => Err(TypeError::new())
    }
}

In the end it will not change how this fields are matched, but it reduces the boilerplate and nesting required to parse the record into a struct for example. In that case, you probably want to throw an Error if a bin type does not match the struct type. Any thoughts on this?

khaf commented 3 years ago

In my experience, if you want to go that far, you'll have to manage the complexity inside these convenience helper methods, so you'll need three families of them:

value.as_XXX() Result<XXX> will return the value if it is of the desired type, or an error. value.try_XXX(default XXX) XXX will return the value if it is of the desired type, or the default value. value.to_XXX() Result<XXX> will return the value if it is convertible to the target type (including casting and parsing of strings to floats and integers, etc), or an error if all attempts fail.

I'm not against this change, especially since it is relatively simple, but I believe the ultimate solution should be a serializer/deserializer trait to just go around the Bin hash map and directly pass structs to methods and get them back with the values assigned.

jonas32 commented 3 years ago

the as and to makes sense for me. Can you explain what you meant by default value? I think it might be risky to do that. If this method does not fail in case of a type mismatch but return a default (for example 0 as int default), it might return a wrong value to the User without being able to notice that.

I'm not against this change, especially since it is relatively simple, but I believe the ultimate solution should be a serializer/deserializer trait to just go around the Bin hash map and directly pass structs to methods and get them back with the values assigned.

For the deserializer definitely. I'm not sure what a serializer could look like without performance impact and a load of boilerplate for the user like in #97.

khaf commented 3 years ago

The point of try_XXX is exactly for times that either the bin does not exist, or is of an incompatible type. In those cases, the user may want to initialize that field via a default value. Basically it a combination of to_XXX that handles the error internally and returns the default value instead of the error. Aerospike is schemaless, so over time a lot of cruft will accumulate in the records.

Regarding the serializer, I've been thinking of a macro that would take care of it automatically by implementing a variation of the trait in #97. You wouldn't have to write code, just mark your struct with the macro, same as you do with Clone/Debug/etc.

khaf commented 1 year ago

I have implemented the TryFrom trait mentioned in #124 and I think that is the elegant way to handle this issue. What's your opinion?

jonas32 commented 1 year ago

Looks like the easiest solution for this. I'd say this is done then.