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

ClassStuctBinder<T> replace fields of default value #70

Open
gODeaLoAple opened this issue Apr 27, 2023 · 2 comments
Open

ClassStuctBinder<T> replace fields of default value #70

gODeaLoAple opened this issue Apr 27, 2023 · 2 comments

Comments

@gODeaLoAple
Copy link

ClassStructBinder sets values from ISettingNode recursively to provided default value. So it can change even some static readonly field of another class.
For example:

public class MessageFormat
{
    public static readonly MessageFormat Default  = new MessageFormat() { Format = "C" };
    public string Format { get; set; }
}

public class AppSettings 
{
    public MessageFormat DefaultFormat { get; set; } = MessageFormat.Default;
    public MessageFormat SpecificFormat { get; set; } = MessageFormat.Default;
}

So, if we provide object source to ConfigurationProvider:

configurationProvider.SetupSourceFor<AppSettings>(new ObjectSource(new AppSettings()
{
    DefaultFormat = new MessageFormat() { Format = "A" },
    SpecificFormat = new MessageFormat() {Format = "B" }
}));
var settings = provider.Get<AppSettings>();

Console.WriteLine(settings.DefaultFormat.Format); // B
Console.WriteLine(settings.SpecificFormat.Format); // B
Console.WriteLine(MessageFormat.Default.Format); // B

Maybe ConfigurationProvider should create object copy instead of changing existing object or even throw any exception, or print warning about this issue

@kunga
Copy link
Member

kunga commented Apr 29, 2023

This code is wrong even without ConfigurationProvider:

auto settings1 = new MessageFormat();
settings1.SpecificFormat.Format = "B";

auto settings2 = new MessageFormat();
Console.WriteLine(settings.SpecificFormat.Format); // B

Or do you really expect that behaviour if you see that code?

@gODeaLoAple
Copy link
Author

Maybe some users expect that ConfigurationProvider copy an object and call setters on it instead of changing the orgin. For example this case appeared recently: user had set a default value and could not understand why another object changed too. I think we should at least warn users about this non-obvious behavior in some way. Or just copy an object before set values

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