search

JavaMoney / jsr354-api

JSR 354 - Money and Currency API
http://javamoney.org
Apache License 2.0
357 stars 79 forks source link

Make MonetaryAmount Serializable #54

Closed axiopisty closed 8 years ago

axiopisty commented 8 years ago

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 commented 8 years ago

Coverage Status

Coverage remained the same at 76.44% when pulling e4d5d853a608c218c71e0ffacb5e00e6f968c3bf on axiopisty:serializable into 11765077a1a6fe006263559fa0399e063efb3cf5 on JavaMoney:master.

keilw commented 8 years ago

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

shivamgoel commented 8 years ago

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 commented 8 years ago

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

axiopisty commented 8 years ago

Agreed. Thank you for the dialog.

keilw commented 8 years ago

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.