unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
119 stars 42 forks source link

EBNFUnitFormat can not handle chain of TransformedUnit #406

Open unixfan2 opened 10 months ago

unixfan2 commented 10 months ago
val formatter = DefaultFormatService().getUnitFormat("EBNF")
val test = Units.METRE.multiply(2).multiply(2)
println(formatter.format(test))

result: m·2

I guess the reason for that is that EBNFHelper.formatInternal is using "parentUnit = unit.getSystemUnit()" instead of the direct parent in the last else block for handling transformed units. Maybe changing that would also make all the dirty hacks in that section obsolete.

unixfan2 commented 10 months ago

Please note that the local formatter is implemented similarly. Maybe some redundant stuff can be merged.

keilw commented 10 months ago

Thanks, could you try, how UCUMFormat treats this as a comparison?

unixfan2 commented 10 months ago
val indriyaFormatter = DefaultFormatService().getUnitFormat("EBNF")
val ucumFormatter = UCUMFormatService().unitFormat
val indriyaUnit = Units.METRE.multiply(2).multiply(2)
val ucumUnit = UCUM.METER.multiply(2).multiply(2)
println("indriyaFormatter/indriyaUnit:" + indriyaFormatter.format(indriyaUnit))
println("ucumFormatter/indriyaUnit:" + ucumFormatter.format(indriyaUnit))
println("indriyaFormatter/ucumUnit: " + indriyaFormatter.format(ucumUnit))
println("ucumFormatter/ucumUnit: " + ucumFormatter.format(ucumUnit))

Result:

indriyaFormatter/indriyaUnit:m·2
ucumFormatter/indriyaUnit:(m.2).2
indriyaFormatter/ucumUnit: m·2
ucumFormatter/ucumUnit: (m.2).2

The UCUM formatter seems to handle that nicely.

I tried my suggested fix for the EBNF stuff and it works fine in my initial tests, but I'm too lazy to apply for a JCP/JSR membership just for creating a PR. But my fix is only to set the parentUnit to the parent unit in the case of a transformed unit + deleting the workaround stuff in that section (hopefully there are unit tests which cover them, but the unit tests work as without the fix).

keilw commented 10 months ago

Thanks for trying. As they all use resource bundles underneath the hood, there could be some inspiration or patch based on UCUMFormat. The latter does not need JCP membership. For the odd PR I also think that is fine, and you could propose one without a JCP membership. If someone contributes more often, or many lines of code, then it would be better.