iddm / serde-aux

An auxiliary serde library providing helpful functions for serialisation and deserialisation for containers, struct fields and others.
MIT License
152 stars 26 forks source link

feat: add `get_default_empty_record` #40

Closed lucatrv closed 6 months ago

lucatrv commented 6 months ago

Summary by CodeRabbit

coderabbitai[bot] commented 6 months ago

Walkthrough

This update introduces a new function get_default_serde_record to the library, enhancing the serde::Deserialize trait implementation. It's designed to simplify the process of defining default records for structs, particularly useful when working with custom default definitions. The addition is complemented by test cases to ensure its functionality and reliability, marking a significant improvement in the library's usability for developers dealing with serialization and deserialization.

Changes

File(s) Change Summary
src/serde_introspection.rs Added get_default_serde_record function and test cases for enhanced usability.

🐇✨ In the land of code and byte,
A rabbit hopped into the light.
With a flick and a hop, a new function's born,
To ease the devs' toil from dusk till morn.
"Fear not," it squeaked, with a twinkle in its eye,
"For I bring ease to serialize, no longer shall you sigh."
🌟📜

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips ### Chat There are 3 ways to chat with CodeRabbit: - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit-tests for this file.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit tests for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository from git and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit tests.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.
iddm commented 6 months ago

@coderabbitai review

iddm commented 6 months ago

Hi @lucatrv ! First and foremost, thank you for your contribution!

Can you please provide a use case for this function to be useful? I am not sure I understand why, for example, and when, I would use it. For example, the code you provided:

#[derive(serde::Deserialize, PartialEq, Debug)]
struct Record {
    #[serde(default = "default_string")]
    label: String,
    #[serde(default = "default_f64")]
    value: f64,
}

fn default_string() -> String {
    String::from("default")
}

fn default_f64() -> f64 {
    1.0
}

let empty_record = get_default_empty_record::<Record>().unwrap();
assert_eq!(
    empty_record,
    Record {
        label: String::from("default"),
        value: 1.0
    }
);

How is it different from this, without using serde at all?

#[derive(serde::Deserialize, PartialEq, Debug)]
struct Record {
    label: String,
    value: f64,
}

impl Default for Record {
    fn default() -> Self {
        Self {
            label: default_string(),
            value: default_f64(),
        }
    }
}

fn default_string() -> String {
    String::from("default")
}

fn default_f64() -> f64 {
    1.0
}

let empty_record = Record::default();
assert_eq!(
    empty_record,
    Record {
        label: String::from("default"),
        value: 1.0
    }
);
lucatrv commented 6 months ago

@iddm I agree that the example above is trivial and can easily be implemented as you showed, however it was intended just to show how the get_default_empty_record method works, not to show a useful use case.

Please consider however that often we deal with long deserialize structs having most fields implementing the Default trait, while maybe one or two use custom types which do not implement it (for instance Result<f64, String>). Of course it is always possible to deal also with these cases as you showed, but it is much simpler and cleaner to rely on Serde's #[serde(default = "path")] field attribute.

In general, any time the #[serde(default = "path")] field attribute is used, it is much easier and safe to use the get_default_empty_record instead of implementing the Default trait separately, paying attention to follow exactly what was specified through Serde's field attributes, which may easily lead to bugs as well.

From another point of view, IMHO what you wrote could also be applied to most helping functions. For instance we could certainly live for instance also without the deserialize_bool_from_anything, as we can implement it in our code as needed, but I think they are all useful helping functions which shorten and simplifies our code.

lucatrv commented 6 months ago

I added some more explanation in the docstring and example.

iddm commented 6 months ago

@iddm I agree that the example above is trivial and can easily be implemented as you showed, however it was intended just to show how the get_default_empty_record method works, not to show a useful use case.

Please consider however that often we deal with long deserialize structs having most fields implementing the Default trait, while maybe one or two use custom types which do not implement it (for instance Result<f64, String>). Of course it is always possible to deal also with these cases as you showed, but it is much simpler and cleaner to rely on Serde's #[serde(default = "path")] field attribute.

In general, any time the #[serde(default = "path")] field attribute is used, it is much easier and safe to use the get_default_empty_record instead of implementing the Default trait separately, paying attention to follow exactly what was specified through Serde's field attributes, which may easily lead to bugs as well.

From another point of view, IMHO what you wrote could also be applied to most helping functions. For instance we could certainly live for instance also without the deserialize_bool_from_anything, as we can implement it in our code as needed, but I think they are all useful helping functions which shorten and simplifies our code.

I see. Thank you for the explanation.

What do you think about renaming the function to get_default_serde_record? To show that it will use the #[serde(default)]. Also, do you mind if we add a clearer text explanation in the document to explain the difference between this function and using the Default trait, the same way you explained it to me? Would it make sense?

lucatrv commented 6 months ago

What do you think about renaming the function to get_default_serde_record? To show that it will use the #[serde(default)]. Also, do you mind if we add a clearer text explanation in the document to explain the difference between this function and using the Default trait, the same way you explained it to me? Would it make sense?

I did as suggested, see if you like it now otherwise please feel free to elaborate further as you will.

iddm commented 6 months ago

Thank you for the contribution and taking time to implement this!

iddm commented 6 months ago

https://crates.io/crates/serde-aux/4.5.0