Implement Serializable?

[kainoa21]

Has there been previous consideration given to making these classes serializable? I was attempting to use this library in a distributed system and being able to serialize and deserialize the citygml classes would greatly improve the usability.

[clausnagel]

No considerations so far because I am using CityGML or CityJSON for de-/serialization. I like your idea and it shouldn't be too hard to implement it. Btw, your are talking about the classes in org.citygml4j.model, right?

[kainoa21]

Correct! I think it is just a matter of adding the interface and potentially adding serialVersionUID. Agreed that CityGML or CityJSON also works for serialization but a bit bloated for inter-process communication. If you are open to it I can submit a PR.

[clausnagel]

Yes, a PR would be great. I think, the root interface of all model classes is org.citygml4j.model.common.base.ModelObject. So it should be sufficient to let this interface extend Serializable.

I am not sure about the serialVersionUID field. Is it required for every non-abstract class? Or is it sufficient to be present for the (abstract) root class of an inheritance chain to be considered for all classes in that inheritance chain? If the latter is possible, this would substantially reduce the effort.

[kainoa21]

I am not an expert in this space, but my sense is that any class which contains fields that you intend to be restored after deserialization need to be marked as Serializable otherwise the default empty constructor will be called. So I think this means that most of the classes in the org.citygml4j.model will need to explicitly implement Serializable (and therefore should probably maintain their own serialVersionUID).

[clausnagel]

I did a quick test. It seems to be sufficient to let the interface org.citygml4j.model.common.base.ModelObject and the abstract class org.citygml4j.model.module.AbstractModule extend resp. implement the Serializable interface. After that I was able to serialize and deserialize a citygml4j object. Well, I have not worked with Java serialization before so I was surprised to learn that it is quite slow.

Afterwards, I added a serialVersionUID to org.citygml4j.model.gml.base.AbstractGML which is a base class for many citygml4j classes. With that change, I was not able to deserialize the object from my previous test anymore. So it seems to be sufficient to use serialVersionUID on root classes of an inheritance chain.

[mprins]

You should read https://stackoverflow.com/questions/285793/what-is-a-serialversionuid-and-why-should-i-use-it

Afterwards, I added a serialVersionUID to org.citygml4j.model.gml.base.AbstractGML which is a base class for many citygml4j classes. With that change, I was not able to deserialize the object from my previous test anymore. So it seems to be sufficient to use serialVersionUID on root classes of an inheritance chain.

this conforms to the spec; because you now have a different version of the object so it is assumed incompatible.
if you always want to create compatible java serializations each generated class should have a serialVersionUID, it can be 1L, otherwise data may not be deserialized in a different JVM

[clausnagel]

@kainoa21 Sorry for not being very active on this lately. I pushed the implementation of the Serializable interface to the serializable branch for testing. It required just a small change to the model classes as written above. And when using buffered input and output streams, serialization performance is just fine. So feel free to test the branch and provide feedback or edits.

@mprins Thanks for your input.

[clausnagel]

merged serializable branch into master (7c32e28)

[kainoa21]

This is great @clausnagel and I have verified that is working for my use case. Thank you for enabling this!