mff-uk / odcs

ODCleanStore
1 stars 11 forks source link

When deserialization fails, default instead of null value should be provided #1292

Open tomas-knap opened 10 years ago

tomas-knap commented 10 years ago

Suppose that user:

1) Serializes configuration for DPU template A 2) Adjusts the DPU configuration class for DPU template A, loads it to odcs 3) Tries to deserialize the previously stored configuration (with the previous version of the config class) 4) As a result, all fields except of the new one should be deserialized properly. The new field which was not available in the previous configuration version is set to null. Why the default value is not used for such attribute?

Better behaviour: When there is an error when deserializing certain attribute, instead of using null and calling onDeserialize(), it would be better to set the default value and then call on Deserialize().

How to do it: When there is an error deserializing certain attribute, you may create new instance and try to get, via reflection, the default value for that attribute.

Motivation: onDeserialize() can serve in the same way, but it duplicated the effort of DPU developer, he has to initialize the attribute on two places which may introduce errors.

tomas-knap commented 10 years ago

Do we need onDeserialize() method then?

skodapetr commented 10 years ago

Actually imho yes .. or at least with design of some configurations. Imagine the situation .. You have extractor that extracts from multiple sources .. and those sources are serialized into the String. Then you decide to add some metadata to them .. so you do not add new values but you modify the existing ones. Also you may wan't to store then into List<> (or some complex class) so you need onDeserialize method to copy the data from string into the List<>. Of course you can do this inside the DPU before the configuration is used .. and also inside the dialog before the configuration is used ... or you can do it on one place.

And still you may want to have several transient fields in your configuration and somehow compute them before/after serialization from serialized data.

tomas-knap commented 10 years ago

In case of deserializing configuration from XStream, it works in a way that firstly the non-param constructor is called, then the fields are set, right?

On Mon, Mar 10, 2014 at 8:35 AM, Petr Škoda notifications@github.comwrote:

Actually imho yes .. or at least with design of some configurations. Imagine the situation .. You have extractor that extracts from multiple sources .. and those sources are serialized into the String. Then you decide to add some metadata to them .. so you do not add new values but you modify the existing ones. Also you may wan't to store then into List<> (or some complex class) so you need onDeserialize method to copy the data from string into the List<>. Of course you can do this inside the DPU before the configuration is used .. and also inside the dialog before the configuration is used ... or you can do it on one place.

And still you may want to have several transient fields in your configuration and somehow compute them before/after serialization from serialized data.

— Reply to this email directly or view it on GitHubhttps://github.com/mff-uk/ODCS/issues/1292#issuecomment-37158584 .

skodapetr commented 10 years ago

XStream do deserialization .. and set whatever he can .. we watch this process and take a note of all set fields. Then we use reflection to get list of all fields in configuration and if there are fields that has not been set, then we create default configuration (empty ctor) and use getters/setters to copy required values.

tomas-knap commented 10 years ago

Ok. So the basic advice is - if you need any initialization (apart from the setting of the initial value for persisted fields), which should be done when deserialization is done or when the new object is built, put it into onDeserialize method, so that the code is not on two places. Maybe we should call it postInitialization() instead of onDeserialize()?

On Mon, Mar 10, 2014 at 8:53 AM, Petr Škoda notifications@github.comwrote:

XStream do deserialization .. and set whatever he can .. we watch this process and take a note of all set fields. Then we use reflection to get list of all fields in configuration and if there are fields that has not been set, then we create default configuration (empty ctor) and use getters/setters to copy required values.

— Reply to this email directly or view it on GitHubhttps://github.com/mff-uk/ODCS/issues/1292#issuecomment-37159376 .