feat: platform value abstraction #805
Conversation
| use crate::Value; | ||
|
|
||
| impl Value { | ||
| pub fn string_representation(&self) -> String { |
There was a problem hiding this comment.
shouldn't be implemented std::fmt::Display instead?
There was a problem hiding this comment.
hmmm good point, but I'll do both
There was a problem hiding this comment.
my brother in Christ our APIs are obese with tons of highly specialized methods and spread across multiple files for a one single type, I truly regret that I used impl GroveDb once and now we have large impl Drive and now this type becomes huge; having a standard API is a blessing and with Display you'll have a .to_string() for free, I see not a single reason to have this method
There was a problem hiding this comment.
okay will remove it from being public then.
| /// A representation of a dynamic value that can handled dynamically | ||
| #[non_exhaustive] | ||
| #[derive(Clone, Debug, PartialEq, PartialOrd)] | ||
| pub enum Value { |
There was a problem hiding this comment.
maybe it's for the next step, but I would think of introducing the Identifier variant as this one has sometimes a different treatment, especially in terms of displaying (or) serializing.
actually, the concept of the package is similar to the projects I was working on someday ago: https://github.com/qrayven/oxygen/blob/main/src/types/value.rs#L21
| /// let r_value : u64 = value.as_integer().unwrap(); | ||
| /// assert_eq!(17, r_value); | ||
| /// ``` | ||
| pub fn as_integer<T>(&self) -> Option<T> |
There was a problem hiding this comment.
I see many conversions baked into Value type, but how about reducing methods' number and use a trait for it called TryFrom?
For instance, as_ family could be done as TryFrom<&'a Value> for sometype and then sometype::try_from(&value) is available, for owning conversions TryFrom<Value> may be implemented.
There was a problem hiding this comment.
I thought about that, but I personally felt this was better, because otherwise you would need to type your variables a lot.
Like let a : Option<String> = value.try_from()?
compared to let a = value.get_optional_string()?
I personally prefer the second one, because you have the value, and then you express what you want from it.
I do think though it's probable a good idea to implement try_from's as well.
QuantumExplorer
dismissed stale reviews from fominok and qrayven
via
17ff1b8
QuantumExplorer
changed the title
Issue being fixed or feature implemented
This PR removes Cbor Values from RS Drive and replaces them with new "Platform Values" that are more powerful and versatile.
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only