grollmus/BuildNotifications

Implicitly synchronize values between Option ViewModel and Model

Opened this issue · 0 comments

Well yeah, I guess that's the difference to the way I think is more efficient to program and in the end the approach ReflectSettings uses.
My though process is as follows:
Let's say you have a property in your ViewModel like this:

public string SomeProperty {
   get => Model.SomeProperty;
   set => Model.SomeProperty = value;
}

With no need to explicitly save values, because each value is directly written to the underlying model.
Now, with our Option approach, I could imagine something similar to this:

public TextOptionViewModel SomePropertyOption { get; } = TextOptionViewModel.FromBinding(Model, m => m.SomeProperty);

And the TextOptionViewModel takes care of setting the SomeProperty value. Which makes sense, so the TextOptionViewModel can take care of all the "is my input value correct?" "Does it exceed min/max?" "AvailableValues etc.".

If you WANT an explicit save functionality, I'd suggest to shadow copy the model. So something like:

var copy = (Model) model.MemberwiseClone();
// modify copy
// persist:
JsonConvert.Populate(JsonConvert.Deserialize(copy), model);

This way you bypass all the clutter code that you have to do.
This is what I had in mind when I said "[have this] done implicitly via serialization"

I guess I just personally hate the fact, that I have to write more than 1 line in my ViewModel for something so trivial.
Or more than 1 property.

Maybe I'm wrong and people generally prefer it that way, I just can't seem to find a compelling argument to convince me that explicitly writing the "How the value is loaded, how it is given outside and how it's persisted" is better than just "do it the same way like all the other values. You know how".
In modern Web Dev it is done all the time.
Anyway, I leave this decision to you. I don't have a strong grudge against doing it the way you wrote. So either way is fine by me.

Originally posted by @BerndNK in #132