Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More to_type functions for Value #99

Closed
jonas32 opened this issue Feb 18, 2021 · 5 comments
Closed

More to_type functions for Value #99

jonas32 opened this issue Feb 18, 2021 · 5 comments

Comments

@jonas32
Copy link
Contributor

jonas32 commented Feb 18, 2021

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
Copy link
Collaborator

khaf commented Feb 22, 2021

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
Copy link
Contributor Author

jonas32 commented Feb 23, 2021

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
Copy link
Collaborator

khaf commented Feb 23, 2021

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
Copy link
Collaborator

khaf commented Apr 29, 2023

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
Copy link
Contributor Author

jonas32 commented Apr 29, 2023

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

@khaf khaf closed this as completed May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants