Avoiding serialization of default values is something good generally speaking.
I think you mean good, from a performance point of view. However, I am not convinced that it is good from a usability point of view.
I think what made this problematic has to do with the concept ‘default value’ being used for two things at the same time in this discussion: 1) the default value of a datatype, which can be relied upon as a performance optimization (and this is what 2.8 does) and 2) the default value of a parameter, which can be used as a convenience mechanism for the user when adding new nodes to flows.
Optimizations that ignore serializing/deserializing default(T) values are fine. If done correctly they should never affect functionality, just improve performance.
So what is the ‘default value of a parameter’ then? It is a different concept than default(T). It should be a value that is assigned to the parameter when adding a new node to a graph (which I presume is what the OnCreate() solution will do). It should ideally also be accessible to a ‘reset parameter to default’ UI feature, and provide grey/white coloring to tell whether the current value matches the ‘default value of the parameter’.
If people were to use the ‘default value of a parameter’ feature a lot, then the don’t-serialize-default(T)-values optimization becomes less effective. However, that is probably not a big problem in practice.
Based on this, I do not like solution 1. It looks nice on the surface, but it has the problem with tying default(T) and default-value-of-parameter together and this causes problem retroactively when changing the default-value-in-parameter in the C# code after a node has already come into use.
I like solution 2, because it is small and it does not come with any gotchas. We will likely go with that approach. At this point I must double-check: OnCreate() isn’t called when deserializing a saved node, is it? because if it is called at each deserialization then this too would suffer from the same problem as solution 1.
As for solution 3, that is also a viable option to us, but we would prefer not to have any modifications in core FlowCanvas – which is why we solution 2.
Login
Register
By registering on this website you agree to our Privacy Policy.