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

Make MonetaryAmount Serializable #54

Closed
wants to merge 1 commit into from

Conversation

axiopisty
Copy link

I'm submitting this pull request to make the MonetaryAmount interface Serializable.

I realize the authors of the specification may have already discussed this and decided purposefully to not make the MonetaryAmount interface Serializable, but I have not been able to find any discussion about this. If so, then I would like to start a discussion, even if only for my own understanding, as to why it was decided that MonetaryAmount itself would not be Serializable.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.44% when pulling e4d5d85 on axiopisty:serializable into 1176507 on JavaMoney:master.

@keilw
Copy link
Member

keilw commented Aug 24, 2016

Thanks, but there have been discussions (also in downstream projects, so you may not find it in JIRA) both for JSR 354 and 363 about making API elements Serializable or not. As JSR 354 also used a few aspects and inspirations by JSR 310 (java.time) its closest equivalent https://docs.oracle.com/javase/8/docs/api/java/time/temporal/TemporalAmount.html does not extend Serializable either. Instead implementing classes like https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html do. The same is followed by JSR 354 where e.g. concrete classes like Money implement Serializable in addition to MonetaryAmount but that as such doesn't. HTH, Werner

@keilw keilw closed this Aug 24, 2016
@shivamgoel
Copy link
Contributor

As a best practice, concrete classes only should implement Serializable marker interface. When any of your interface does so it passes along Serializable downstream forcibly and many times unrealized.

I would recommend to go through some content regarding this area and may be tell us more about the reasoning behind making this interface extend Serializable.

Go through this as well http://efectivejava.blogspot.com/2013/07/effective-java-item-74-implement.html

@keilw
Copy link
Member

keilw commented Aug 24, 2016

Yes, that's what we also followed. Thanks for the pointer.

@axiopisty
Copy link
Author

Agreed. Thank you for the dialog.

@keilw
Copy link
Member

keilw commented Aug 24, 2016

Thanks, it was very welcome as someone pointed out, JSR 374 (JSON-P 1.1) used it in a concrete class uder the API, Both not good to do in the API, especially if you have Java 8 and interfaces may define "default" implementations, too.

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