[JAVAMONEY-155] make ConvertNumberValue public Created: 25/Sep/15  Updated: 27/Sep/15

Status: Open
Project: javamoney
Component/s: Impl: RI
Affects Version/s: 1.0
Fix Version/s: None

Type: Improvement Priority: Major
Reporter: msgilligan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: 2 hours
Time Spent: Not Specified
Original Estimate: 2 hours


 Description   

When extending NumberValue, implementing numberValue(Class<T> numberType) and numberValueExact(Class<T> numberType) is non-trivial.

For those willing to add a dependency on the RI, it would be nice to be able to use the ConvertNumberValue enum. Can it be made public?



 Comments   
Comment by otaviojava [ 27/Sep/15 ]

I don't think it's the good idea:

  • This class is not the goal of the API
  • Once this class public we need to take care any refactoring, in other words, any improvements or change in this class it will slower.
  • To remove this class one day if public it will spend more time, one to deprecated it and then to finally remove it.
Comment by keilw [ 27/Sep/15 ]

Well the same goes for other RI elements like MonetaryUtil which was public and has to be restored, or if it was to stay in a similar form MonetaryOperators and other RI level public elements. I don't see if we add all sorts of stuff in a function package why this should not be available the same way.

Comment by msgilligan [ 27/Sep/15 ]

The problem is that any class implementing NumberValue is required to implement

<T extends Number> T numberValue(Class<T> numberType)

which is essentially a requirement to convert to any of the built-in number types. So every implementation is being forced to implement this functionality.

Perhaps making ConvertNumberValue isn't the best API or implementation to make public. But something should be provided by the RI and possibly even specified in a future version of the API. In fact, I'd go so far as to say that an arbitrary Number to Number conversion should be included in the JDK itself.

Comment by msgilligan [ 27/Sep/15 ]

The ideal solution might be something like this:

1. NumberValue (or a provided subclass) would require a no-parameters numberValue() method that would return a Number of the same type that getNumberType() returns. This would be the default or preferred representation of the internal type.

abstract public Number numberValue();

I'm not sure whether there would need to be a no-parameters version of numberValueExact or if we could safely assume that the preferred conversion would always be exact.

2. This provided abstract base class would provide an implementation of numberValue(Class<T> numberType) similar to:

public <T extends Number> T numberValue(Class<T> numberType) {
        return ConvertNumberValue.of(numberType, numberValue());
}

Summary

Implementing classes would provide a method to convert to the optimal standard Number type. The framework itself would provide the method that converts to any of the standard subtypes. This would make creating an implementation of NumberValue much easier as only a conversion to a single type would be required. In most cases I would imagine that type would be Long, BigInteger, or BigDecimal.





[JAVAMONEY-160] equal can return false for the same CurrencyUnits Created: 19/Feb/16  Updated: 16/Mar/16

Status: Open
Project: javamoney
Component/s: Impl: RI
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: walec51 Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

I have one currency unit created from jackson-datatype-money deserialization and the other one from Monetary.getCurrency(locale). Both refer to USD but when I try to compare them using equal I get false.

This behavior is very surprising. The reason for this was that one MonetaryUnit was a BuildableCurrencyUnit and the other one was a JDKCurrencyAdapter and their equals implementations are not symmetrical.

JDKCurrencyAdapter has
if (obj instanceof CurrencyUnit) { ...

BuildableCurrencyUnit has
if (obj instanceof BuildableCurrencyUnit) { ...
this is wrong and it should be implemented as in JDKCurrencyAdapter



 Comments   
Comment by keilw [ 19/Feb/16 ]

Interesting, we may have a look into it (given both RIs are currently working on a maintenance pack it sounds like good timing) but keep in mind, e.g. Java's own Integer and BigInteger both created with the same value 1, 2 or any number never return true on equals() either. So it could be a "works as Java intended" case, but if there's a domain specific deviation we may accomplish here we'll give it a try.

Comment by tommy_ludwig [ 16/Mar/16 ]

I also have run into this problem comparing MonetaryAmount objects deserialized from JSON using jackson-datatype-money with ones instantiated using Money.of(number, currency). You can see a demonstration of this in JUnit tests in this gist. I understand the precedent of the Number classes, but two Money objects being compared should not return different values depending on the order of the equality check. The equals method states that it should be symmetric.

I think it is mostly a problem of usability and consistency. There are workarounds (Money#compareTo), but they make things a bit ugly and it is just too easy to use "the wrong thing" (in this case equals). Money#isEqualTo works if you know the currencies are the same or are okay handling the MonetaryException in case of mismatched currencies, but I don't know that and want to compare equality in a stream.





[JAVAMONEY-142] Define some tests as "integration-test" Created: 27/Jul/15  Updated: 23/Aug/15

Status: Open
Project: javamoney
Component/s: Impl: RI
Affects Version/s: 1.0
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: keilw Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to JAVAMONEY-125 Add missing tests Open
Tags: test

 Description   

Some CI servers like Circle-CI call mvn integration-test by default. For those tests in need of external services by ECB, IMF, Yahoo, etc. we should try to rebrand them into "Integration Tests" and run them separately or optionally where necessary. See http://stackoverflow.com/questions/1399240/how-do-i-get-my-maven-integration-tests-to-run






Generated at Thu May 05 23:30:08 UTC 2016 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.