[UNITSOFMEASUREMENT-70] Add five methods in Quantity interface Created: 31/Oct/14  Updated: 03/Nov/14  Resolved: 02/Nov/14

Status: Resolved
Project: unitsofmeasurement
Component/s: API, RI
Affects Version/s: None
Fix Version/s: 0.7

Type: Improvement Priority: Minor
Reporter: otaviojava Assignee: otaviojava
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Tags: arithmetic, design
Epic Link: Design
Sprint: Early Draft

 Description   

The propose is add five new comparatives methods in Quantity interface:

boolean isGreaterThan(Quantity<T> quantity);
boolean isGreaterThanOrEqualTo(Quantity<T> quantity);
boolean isLessThan(Quantity<T> quantity);
boolean isLessThanOrEqualTo(Quantity<T> quantity);
boolean isEquivalentTo(Quantity<T> quantity);

The implementation should convert to the same unit and than compare.
So:

Quantity<Time> oneHour = ...
Quantity<Time> sixtyMinutes = ...

Should return true, when:

oneHour.isEquivalentTo(sixtyMinutes);

In another JSRs such JSR 354 and JSR 310 there are comparatives methods:



 Comments   
Comment by keilw [ 31/Oct/14 ]

Looking at even the FastMoney implementations of these methods in Moneta like:

    public boolean isLessThan(MonetaryAmount amount) {
        checkAmountParameter(amount);
        return getBigDecimal().compareTo(amount.getNumber().numberValue(BigDecimal.class)) < 0;
    }

None of them seem to get to a result without BigDecimal.
That looks like a waste for the general API level Quantity or Measurement depending on where operations end up.

Keep in mind, LocalDateTime in JSR 310 are IMPLEMENTATION classes of an RI boggled up with an anemic hard to distinguish API, but the few interfaces like Temporal, TemporalAmount or TemporalUnit can be seen as minimalistic API, and neither of these contain more than a few like plus() or minus() (a Scala style, that has not been accepted anywhere else, hence 354 or this JSR prefer the pattern by java.math classes )
So it feels, like keeping them e.g. in implementation classes like DecimalQuantity or no further than AbstractQuantity looks better. Happy to see what other EG members feel, but based on mostly RI locations in other mentioned JSRs I'm hesitant to think it adds enough value to the API. In 354 it is mostly for precision math with BigDecimal, something we don't have in ME.

Comment by desruisseaux [ 01/Nov/14 ]

Why not just implement the int compareTo(Quantity<T>)) method from the Comparable interface instead?

Comment by otaviojava [ 01/Nov/14 ]

About Comparable interface, we already had this discussion and the answer was not.

Comment by desruisseaux [ 02/Nov/14 ]

At the time we had that discussion, they were no proposal to add comparison methods in Quantity. Now this proposal change the situation, we should at least revisit the arguments against Comparable. I have heard so far:

  • Quantity shall be implementable by Enum. Okay. But most Quantity methods like getValue(), getUnit(), add, multiply make no sense for enumeration like LIGHT or HEAVY. Is Quantity really the right interface for those enumerations?

Does someone remember other arguments?

If the decision still to not extend Comparable and to provide different comparison purpose, then I think that we should provide 5 methods. We should provide only 1, maybe something like compareTo but returning an Enum instead than an int. The Enum values could be LESS, EQUIVALENT (not exactly the same than "equals" in the Java sense), GREATER or NOT_COMPARABLE. Having 5 separated methods double the API weight and is a performance problems, since sorting Quantity objects requires calling 2 or 3 methods for many pair of objects (e.g. isGreater, followed by isEquals or isLess if the former returned false. Invoking a third methods may be necessary if we want to support non-comparable values like NaN).

Comment by otaviojava [ 02/Nov/14 ]

It could not be useful to user, how just comparator that return a boolean.
How add these methods it can impact in performance?
Probably, the implementation will use some thing like compareTo.

To sort already exist the Comparator interface.
In SE implementation, for example, we have the QuantityFunctions that has five kinds of sort, I believe it don't will problem put these comparators, doing the downgrade, in ME platform (one or all).

Comment by otaviojava [ 02/Nov/14 ]

@keilw , Sorry I don't understand your point, I am not talking about precision or not, just method to do comparatives. Like this:

Quantity<Time> oneHour = ...
Quantity<Time> sixtyMinutes = ...
if (oneHour.isEquivalentTo(sixtyMinutes)) {
...
}

Quantity<Length> grams = ...
Quantity<Length> kiloGramas = ...

if (kiloGramas.isLessThan(grams)) {
...
}

The implementation should convert and than compare the value.

Comment by keilw [ 02/Nov/14 ]

No, Quantity in its current form is a Magnitude (or the Quantity, OpenXC has) while the abstract base type Measurement (that's what OpenXC calls Unit ) can and should be implemented by Enums, too.

In theory Quantity could also extend Comparable, but see JSR 310 most of its concrete classes do, not the more or less API classes like TemporalAmount.

Comment by desruisseaux [ 02/Nov/14 ]

Otavio: about performance, I'm not saying that adding those methods would degrade performance of other methods. I'm saying that a bunch of methods like:

  • boolean isGreaterThan(...)
  • boolean isLessThan(...)
  • etc.

is less performant in some scenarios than a single method like:

  • int compareTo(...) (or any other methods returning an int or an Enum if we do not want to clash with Comparable).

Sorting is not the only example. Any algorithm who needs to know if A is less, equals or greater than B will be impacted in the same way.

Comment by otaviojava [ 02/Nov/14 ]

hum,
Understood, thank you to explain me, but the focus of this method is just a single useful comparative and get the code more expressive.

For example:

This code is more expressive:

if (kiloGramas.isLessThan(grams)) {
...
}

Then this:

if (kiloGramas.compareTo(grams)  <  0) {
...
}

Just to follow anothers new APIs, that are using method as DSL language.
In Enterprise App, we always need to do a comparatives like these.
To sort functions I believe the best strategy is implements the Comparator interface.

Considering the boolean in JVM does not exist and will be an int, I believe it will not lost performance, maybe in the worse scenario a slight loss of performance.

Comment by keilw [ 02/Nov/14 ]

Don't want to shut this down or resolve with "won't fix" but until the Quantity design and scope is finalized this makes no sense.
If the current trend, that Quantity shall keep all of its methods and Measurement is practically submerged into it, hence drop API support for non-numeric custom types (which would often be modelled as Enum) is executed, there could be a viable alternative to extend Comparable<Quantity> and IMHO that would seem a bit leaner.
If implementations really need this, feel free to add it to AbstractQuantity in the SE port to see what value it adds.

Comment by otaviojava [ 02/Nov/14 ]

Ok, I specializes to SE.

Comment by otaviojava [ 02/Nov/14 ]

Just to SE





[UNITSOFMEASUREMENT-66] Add asType() to Quantity Created: 20/Oct/14  Updated: 20/Nov/14  Resolved: 20/Nov/14

Status: Resolved
Project: unitsofmeasurement
Component/s: API
Affects Version/s: None
Fix Version/s: 0.7

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

Issue Links:
Related
is related to UNITSOFMEASUREMENT-77 Could QuantityFactory safely return s... Open
Tags: arithmetic, quantity, types
Epic Link: Design
Sprint: Early Draft

 Description   

To provide type checking and a safer alternative to a cast, Quantity should also have an asType() method.



 Comments   
Comment by desruisseaux [ 01/Nov/14 ]

I think that casting to an explicit type like (Length) or (Mass) would be cleaner. It still not clear to me why the implementation can not create instance of the right type. It is because the Unit implementation has no Class<Q> information associated with it?

Comment by keilw [ 02/Nov/14 ]

Signature

 <T extends Quantity<T>> T asType(Class<T> type);

proposed instead

Comment by keilw [ 02/Nov/14 ]

Because by casting you can do any "crap", see a patch of https://github.com/unitsofmeasurement/uom-demos/blob/master/domain/health/src/main/java/tec/uom/demo/health/BMIDemo.java

		//Quantity<Volume> squareHeight = height.multiply(height).asType(Volume.class);
		Quantity<Volume> squareHeight = (Quantity<Volume>) height.multiply(height);

The first line won't run without a runtime exception, the second one will, though the result is wrong.
So asType() has its value, though it may not be usable in every situation, there could be a need for an explicit cast, but for "standard situations" mainly what OSGi Measurement would also cover, we have a more typesafe approach even at runtime.

Comment by keilw [ 02/Nov/14 ]

Trying to cast to Volume:

Volume squareHeight = (Volume) height.multiply(height);

will fail for another reason:

Exception in thread "main" java.lang.ClassCastException: tec.units.ri.quantity.DoubleQuantity cannot be cast to javax.measure.quantity.Volume
	at tec.uom.demo.health.BMIDemo.main(BMIDemo.java:42)

And IMHO the type of the general purpose operations is likely doomed to fail a typesafe cast forever,
You could (since they are all derived from Quantity) try to pass in Length or Mass but the operation cannot instanciate any mutual concrete class except performing "Proxy Voodoo" and that is not possible on ME.

Hence, PoC is necessary for a substantial change to QuantityFactory in https://github.com/unitsofmeasurement/unit-api/tree/master/src/test/java/javax/measure/test first. Without Dynamic Proxies, etc.

Jean-Marie's proposal for something like Area = Length.multiply(Length) would work, but require explicit implementation of all these methods in a concrete class, see https://github.com/unitsofmeasurement/unit-api/blob/master/src/test/java/javax/measure/test/quantity/DistanceQuantity.java

Comment by otaviojava [ 03/Nov/14 ]

About the this method, I just would like to change the exception ClassCastException to anything more expressive maybe:
InvalidUnitConvertionException

I prefer this strategy than specific methods, byond more implementations, we will limited the another implementations to just the API and Have the multiply and Quantity and in specific interface, IMHO no make sense.

Comment by keilw [ 03/Nov/14 ]

Well we should consider this carefully. The exception is thrown by classes like AbstractUnit so it is within our control, but similar to JSR 354 we prefer to reuse standard Java exceptions, i.E. throw an ArithmeticException in other cases, not some UnitArithmeticException

There is a UnconvertibleException (also extending the RuntimeException MeasurementException) but its current definition is closely bound to actual UnitConverter and conversion. Again, let's see, what others say, we should try to keep the number of special exceptions at a decent level, not trying to get to a level of UCAR (based on JSR 108) with 24 exceptions, and that API doesn't even cover the combination of units and values like ours, OSGi Measurement or ICU4J :-|

Comment by otaviojava [ 03/Nov/14 ]

I believe IllegalStateException, is better than ClassCastException and already is on JDK:
http://docs.oracle.com/javase/7/docs/api/java/lang/IllegalStateException.html

Comment by keilw [ 03/Nov/14 ]

IllegalState makes no sense here, at most I could imagine IllegalArgumentException which is used in one or two places already.

JSR 354 MonetaryAmount states, "Arithmetic operations should throw an ArithmeticException", but e.g. one of those "compareTo() convenience methods" you also suggested for 363 @throws MonetaryException if the amount's currency is not equals to the currency of this instance.

The general problem sounds similar, but let's discuss, if we leave CCE, consider IllegalArgumentException a better choice or used MeasurementException the equivalent to MonetaryException here?

Comment by keilw [ 03/Nov/14 ]

Also this is a minor detail, the real challenge lies in a solid proof (JUnit tests in API, a selected number of concrete quantities can be found here: https://github.com/unitsofmeasurement/unit-api/tree/master/src/test/java/javax/measure/test) that changing at least QuantityFactory to return interfaces like Length rather than Quantity<Length will work without runtime exceptions.
Mostly ClassCastException btw. thus, IMHO asType() does it right at the moment. Class.cast() also does the same, and since the underlying Unit.asType() performs a cast to a desired unit, if that fails for whatever reason, you also get a ClassCastException so it seems more consistent, thatn throwing 2 different exceptions

Comment by keilw [ 20/Nov/14 ]

Actual issue was fixed. Returning a different element from QuantityFactory or passing sub-types of Quantity instead of the parent interface to arithmetic operations is a different question that deserves a different task or multiple tasks.





[JAVAMONEY-42] Add Percent Operation Created: 09/May/13  Updated: 05/Feb/15  Resolved: 30/Aug/13

Status: Closed
Project: javamoney
Component/s: Impl: RI
Affects Version/s: 0.4-EDR1
Fix Version/s: 0.5

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

Tags: arithmetic, core

 Description   

Based on Chris' suggestion
1) Do we need to model the concept of a percentage? I think this would make a lot of money operations more readable (especially GST / VAT), and it's a candidate for being an immutable type itself.

and existing approaches to that in Eclipse UOMo Business let's try to create a reusable Percent operation ideally based on the MonetaryOperation/MonetaryFunction paradigm the JSR already has.






Generated at Sun Aug 30 18:39:00 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.