Re: [Joda-money-interest] A quick "review"...

[Stephen C. <sco...@jo...>]

Benoit Xhenseval wrote:
> ·         It can handle replacement currencies: e.g. old Turkish Lira to 
> New Turkish Lira; Belgian Franc to Euro where the rate from old to new 
> is a FIXED conversion rate.
I wasn't planning any support for the data of conversion rates.

> 2.       It seems that there are a few dependencies which I’d prefer to 
> see separated: Money -> CurrencyUnit -> CurrencyUnitDataProvider.  I 
> feel that the provider should NOT be referenced in the CurrencyUnit. I’d 
> prefer to have Money & CurrencyUnit as a ‘set’.

I'm not sure I follow your goal. I want to ensure that the user cannot 
just create any arbitrary currency. This means that currencies must be 
registered in advance, which I've made pluggable by the provider. I'm 
not sure how you'd make CurrencyUnit not depend on the provider.

> 3.       CurrencyUnitDataProvider should be replaceable as many Banks 
> will have their own data source and will NOT trust anything else; 
> furthermore, currencies do change quickly and doing a new release for 
> this is simply not possible in most cases (eg the introduction of the 
> new Turkish Lira should not require the release of a new version of the 
> application).

Specifying a system property replaces the provider.

> 4.       The current library would not work very well in a distributed 
> environment if the CurrencyUnitDataProvider is tailored.  For instance, 
> say that the server side has its own CurrencyUnitDataProvider that is 
> hooked to the database.  If Money is serialised and sent to a GUI which 
> DOES NOT have access to the DB, it will TRANSPARENTLY revert to use the 
> default CSV file provided by Joda-money.  There should be a mechanism 
> whereby the serialization could take the data as-is from the server.  
> This is also a problem if client and server are using different versions 
> of joda-money.
> So I would drop the readResolve mechanism and accept the serialised version.

Possible, but then there is a mechanism to create currency instances 
that don't match those that are registered. I see the point though - its 
rather like time zone rules.

> 5.       As such, I’d suggest to move the CurrencyUnit .of(Locale...) 
> somewhere else as this is depending on data that could be coming from 
> different provider

Not sure why moving it to another location would be helpful.

> 6.       The storage of the actual value as a long is quite ingenious 
> BUT it is very likely that getAmount is going to be called many times 
> and therefore I’d suggest to keep the result once calculated.

Possible. I suspect most money objects will be short lived.

> 7.       I would suggest to make the Money class more ‘null-resistent’ 
> eg why not returning this if someone calls Money.plus(null) (for 
> BigDecimal).  It would make life easier when doing many calculations.

MoneyUtils provides null-safe operations. Making Joda-Time null-safe was 
a mistake I think.

> 8.       Should the CurrencyUnit have the long name for the 
> currency...eg do you know what AED is?

The problem is that this data isn't available from the JDK until JDK7. 
Now ideally we should get the names from CLDR (along with other stuff), 
but that involves a bit of XML parsing that doesn't greatly excite me 
right now. Feel free to dive in though.

> 9.       Should the CurrencyUnit have a ‘replacement’ currency with a 
> conversion factor? This is very useful when a currency is changed; e.g. 
> DEM to EUR and the conversion rate is a BigDecimal of “0. 51129”

CLDR has currency history info, but not conversion rates. I want to 
avoid conversion rates.

> 10.   I have never heard of Currencies with a numeric code, it is 
> non-standard, probably something for your project. It seems to introduce 
> 2 keys for a currency with the potential inconsistencies. Only the 
> String code is used as key in the code.

The numeric code is part of the ISO standard.

> 11.   On style, I quite like using final as much as possible... Eclipse 
> can enforce that.

I find it quite intrusive :-)

Stephen