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

Derived fields #1689

Closed
Licenser opened this issue Dec 5, 2019 · 5 comments
Closed

Derived fields #1689

Licenser opened this issue Dec 5, 2019 · 5 comments

Comments

@Licenser
Copy link

Licenser commented Dec 5, 2019

Sometimes a struct contains computed fields that logically derive from another attribute of the struct. It would be nice to have a way to express this in serde.

Something like:

#[derive(Serialize, Deserialize)]
struct MyRe{
  src: String,
  #[serde(derived_from="src", with="compile_re")]
  re: Re
}

fn compile_re(src: String) -> Result<Re, Error> {
 Re::compile(src)
}
@H2CO3
Copy link

H2CO3 commented Dec 9, 2019

To me that doesn't look like a job for deserialization at all. Why should deserialization be concerned with compiling a regular expression? It's a massive breach of single responsibility.

This problem can be solved by splitting the type in two: a "pure data" type, which is only concerned with serialization and deserialization of the minimal amount of information necessary to uniquely identify and compute the state of the whole thing, then an "object-like" or "live" type, which constructs itself from the pure data type and has the rich interface you need with potential optimizations, like pre-computed regexes, applied in the constructor (or at least only in this second type).

@Licenser
Copy link
Author

Licenser commented Dec 9, 2019

That is fundamentally wrong. The responsibility of a serializer/deserializer is to convert from memory to wire format, not to cast judgment on which memory format is "ok" and and which isn't. How the function is called is of no relevance here, if it's compile or foogle or from_string do not and should not concern the deserialiser. The only concern is to take some wire format data (or storage format) and apply some transformations and return the memory representation.

Serde already allows all those steps on their own - there is deserialize_with which could call Re::compile at which point it suddenly would be ok I guess? After all that's already implemented.

Re in fact has a as_str function that would allow directly serialising it, so serialize_with and deserialize_with would work in the example (but are not used for the sake of the example as not every in memory format has a convenient as_str function).

Field renaming also works already with rename so that's not out of the responsibility of a deserialiser either.

So the only difference left is that this would populate two in memory fields from a single source field - which, in a way, also already exists with objects that are key/value pairs but is limited to this one specific case.

So all of the components that would create a derive option are clearly the responsibility of the deserialiser - all derive would add is a convenient way to use them together so they don't excluding each other and prevent the deserialiser to do something that isn't it's reasonability - trying to dictate what my memory representation is.

@H2CO3
Copy link

H2CO3 commented Dec 9, 2019

The deserializer doesn't dictate what your memory representation is — good practice and taste does.

Furthermore, asserting that (i) just because something can be done it is desirable to do it and (b) an arbitrary combination of existing components/features of a framework must automatically be sensible is misguided. Lumping these parts together is of course (likely) possible, but it matters how they are used. In fact, in this case, it is exactly the decoupling between them that would get lost and that's what leads to the violation of responsibility boundaries.

@Licenser
Copy link
Author

Licenser commented Dec 9, 2019

That makes absolutely no sense, they are usable together, again the only difference is that a single field on the wire will be deserialised into two fields in memory - there is absolutely nothing that violates responsibility boundaries.

If you have an issue with regexp then use network addresses as an example where you might want to preserve the original string representation along side the numbers so user input like 1.1/24 is preserved but deserialised into two numbers.

The same as DateTime would get serialized to a string and deserialized to a structure.

@dtolnay
Copy link
Member

dtolnay commented Jul 5, 2020

I would prefer not to build anything dedicated to this into serde_derive. For the snippet at the top of this issue, I would recommend handling that kind of thing by writing the Deserialize impl for MyRe which works by delegating to a derived Deserialize impl for a struct containing just the serialized data.

impl<'de> Deserialize<'de> for MyRe {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        let src = MyReRepr::deserialize(deserializer)?;
        let re = Re::compile(&src).map_err(...)?;
        Ok(MyRe { src, re })
    }
}

@dtolnay dtolnay closed this as completed Jul 5, 2020
@serde-rs serde-rs locked and limited conversation to collaborators Jul 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants