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

Deserialize generic objects error message and sample #821

Conversation

rudolfolah
Copy link
Contributor

@rudolfolah rudolfolah commented Jun 21, 2023

Here is one update based on #820

  • added an example to show how to deserialize data into generic objects like lists, dictionaries, or strings, etc.

if we cannot deserialize, we may need to change the target data type that we are trying to deserialize into
generic objects can be lists, dictionaries, strings, etc.
@EdwardCooke
Copy link
Collaborator

One issue I have with changing that exception message is that it can be caused by reasons other than the data type, adding that in there could be just as misleading as it not being there. A simple example:

var yaml = @"
Test: x";
var deserializer = new DeserializerBuilder().Build();
var test = deserializer.Deserialize<Foo>(yaml);
Console.WriteLine(test.Test);

class Foo
{
    private string? _test;

    public string Test
    {
        get
        {
            return _test ?? string.Empty;
        }
        set
        {
            throw new Exception("Setter is broken");
        }
    }
}

It's not the data type that's broken, but instead the property itself. Yes, that exception would be in the inner exception property, but it becomes confusing with the top exception message saying one thing but it being completely different.

@rudolfolah
Copy link
Contributor Author

That's a fair point, I can revert that part of the PR. Let me know if there's anything else needed for the sample code and I'll be happy to update the PR further.

@EdwardCooke
Copy link
Collaborator

A I think reverting the exception messages is all.

@rudolfolah
Copy link
Contributor Author

Awesome, reverted it and updated the PR description.

@EdwardCooke
Copy link
Collaborator

Great! I’ll get this merged in.

@EdwardCooke EdwardCooke merged commit 0d33b5f into aaubry:master Jun 22, 2023
1 check passed
@aaubry
Copy link
Owner

aaubry commented Aug 14, 2023

This feature has been released in version 13.2.0.

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

Successfully merging this pull request may close these issues.

None yet

3 participants