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

Add test case and special handling for Kotlin's object type #215

Closed

Conversation

luqasn
Copy link

@luqasn luqasn commented Feb 22, 2019

Correctly deserialises

object SomeObject

into always the same reference instead of instantiating new ones.

Open to any feedback, not really that familiar with the codebase yet.

@luqasn
Copy link
Author

luqasn commented Feb 22, 2019

Unfortunately, master branch seems broken :(

@apatrida
Copy link
Member

apatrida commented May 5, 2019

What does this do if the values to deserialize differ from the values of the object instance? Does it use the object instance as-is or modify its values?

@luqasn
Copy link
Author

luqasn commented May 19, 2019

It always takes the 'global' object instance and ignores whatever was to be deserialized. I have never used mutable objects because I think that is just bad design, but maybe you are right and it should be supported? Not sure...

@NayanSrivastav
Copy link

any update on this?

@luqasn
Copy link
Author

luqasn commented May 29, 2019

I have read #225 and I think @apatrida raises valid concerns. But as using objects together with data classes with a sealed parent as kind of an enum type is Kotlin idiomatic, I would really expect the jackson-module-kotlin to support this out of the box.

In this use case, the objects being serialized and deserialized do not have any properties anyways, so maybe one could use the custom deserializer only exactly in this case? And throw an exception otherwise? Because I would prefer the deserializer not silently breaking my contract with the runtime (object being a singleton).

Still not great, but maybe better than the current state?

@cowtowncoder
Copy link
Member

Closing in favor of patch #244.

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

Successfully merging this pull request may close these issues.

4 participants