Skip to content
This repository has been archived by the owner on Nov 29, 2024. It is now read-only.

implement Serializable #30

Merged
merged 4 commits into from
Jun 18, 2019
Merged

Conversation

ceefour
Copy link
Contributor

@ceefour ceefour commented Aug 2, 2017

No description provided.

@jeremiehuchet
Copy link
Owner

Hi,

Can you explain why do you need Serializable objects?

@ceefour
Copy link
Contributor Author

ceefour commented Aug 2, 2017

Apache Wicket requires this in order for these objects to be stored on the stateful page store or session store.

@jeremiehuchet
Copy link
Owner

First of all : thank you for sharing :)

I am not comfortable to make this change : it does not add any feature to the library. I understand it is easier to store model objects from the library directly to your session.
However using you own model objects in your session gives you more control. That way you can make them serializable.
You can also get rid of everything you don't need from the library model objects.
And you can make your model perfectly adapted to your functional domain.
It is a pain to translate objects from on model to another. But there are some useful libraries like mapstruct.

If you still need this change, the best I can propose is to keep this PR opened until more people request it.

@ceefour
Copy link
Contributor Author

ceefour commented Aug 2, 2017

I respect your decision.

However, have you thought carefully about the cons of making it Serializable ?

I can always create my own models myself. However sometimes I don't want to, perhaps because it's sufficient to just use it. I may store things like LocalDate on the session, even though I can create my own model in the session that basically wraps a LocalDate, but it'd be extra work without additional benefit.

Having your classes Serializable will provide value & benefit to people who appreciate the convenience of not having to do extra work to wrap your class. I already did the first extra work to make a pull request here.

Sun/Oracle making LocalDate (and lots of other classes) Serializable does not add any date-related feature to that class. But it sure does make other people's lives who need it easier, while not having undesirable effects.

I understand that some changes need careful thought, like bumping the minimum version to Java 8 or adding a 20 MB dependency. But I don't think that's the case here. Unless you provide a strong argument why it's undesirable to have Serializable, I don't see why not.

@jeremiehuchet
Copy link
Owner

Decision is not already made, discussion is opened.
Your arguments are relevant and convinced me :)

@jeremiehuchet jeremiehuchet added this to the next milestone Aug 2, 2017
@jeremiehuchet jeremiehuchet merged commit 98e4554 into jeremiehuchet:master Jun 18, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants