unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
115 stars 40 forks source link

Missing Names, Symbols in static instances #369

Closed GregJohnStewart closed 2 years ago

GregJohnStewart commented 2 years ago

There are a number of static units that are missing name and symbol values. I can probably go in and add them myself and make a PR, but wanted the consensus/ go-ahead from you guys first.

Examples (from the units I currently directly use):

There are way more, seems like most of the ones I use are missing at least their symbol or name.

GregJohnStewart commented 2 years ago

Looking at the code, it seems to be the case that these are units that are derived from others

For instance, GRAM is a derivitave of KILOGRAM, and the addUnit method has no parameters to specify name or symbol.

Ostensibly these parameters will need added and used, though that seems easier said than done.

Perhaps BaseUnit needs a new constructor, something like new BaseUnit<>(String, String, Unit), so we can make an adder along the lines of...

    private static <U extends Unit<?>> U addUnit(String name, String symbol, U unit) {
        U newUnit = new BaseUnit<>(name, symbol, unit);
        INSTANCE.units.add(newUnit);
        return newUnit;
    }

Or, looks like TransformedUnit has the stuff we need, and we just need to use that instead of a more basic unit.divide() (and similar)

andi-huber commented 2 years ago

As far as I understand (I am not involved with the API design), methods ...

public interface Unit<Q extends Quantity<Q>> {

    // Returns the symbol (if any) of this unit. This method returns {@code null} if this unit has no specific symbol associated with.
    String getSymbol();

    // Returns the name (if any) of this unit. This method returns {@code null} if this unit has no specific name associated with.
    String getName();
//...
}

are both rather for framework internal use and probably only declared/needed on base units. You usually need to resort to any of the built-in formatters in order to get human readable output for say Units.GRAM. Apparently Units.GRAM.toString() also works to get you at least the symbol 'g'.

Granted, this is not intuitive and Javadoc above is rather vague (but at least not wrong).

That said, if adding missing symbols and names does not break the framework, then I don't see any reason to not do it. Otherwise if it breaks the framework or if its hard to do for technical reasons, we should at least clarify above Javadoc.

I don't have enough insight into the formatting stuff, so I cannot give strong advice in any of these directions.

keilw commented 2 years ago

The name is rarely used and addUnit() has no purpose in BasedUnit, it is a method in SystemOfUnits implementations. As for the formatting, you more often get a combined result for units that are derived from others, therefore neither getName() nor getSymbol() are guaranteed to return something. They mostly do for well-known units added to the systems, but you always can rely on toString().

GregJohnStewart commented 2 years ago

I'm asking for them to be added, as my use case would greatly benefit from them all being present... toString() has proved not to be sufficient, and I'd rather not have to re-declare a bunch of static members to get this functionality.

To be clear, I am building a system that allows a user to choose their own units, and thus needs proper display of the name and symbol of the unit. toString() gets the point across, but is not enough.

I don't think this is a big ask, this shouldn't be a hard thing to add in, and would add a ton of utility to anyone using the library

GregJohnStewart commented 2 years ago

For now, as a workaround I am currently doing the following. Hacky but it is what it is:

    private static <Q extends Quantity<Q>> Unit<Q> getUnitWithNameSymbol(
        Unit<Q> unit,
        String nameIfNone,
        String symbolIfNone
    ) throws NoSuchFieldException, IllegalAccessException {
        @SuppressWarnings("unchecked")
        Class<? extends Unit<Q>> clazz = (Class<? extends Unit<Q>>) unit.getClass();

        if (unit.getName() == null || unit.getName().isBlank()) {
            Field f1 = clazz.getSuperclass().getDeclaredField("name");
            f1.setAccessible(true);
            f1.set(unit, nameIfNone);
            f1.setAccessible(false);
        }
        if (unit.getSymbol() == null) {
            Field f1 = clazz.getSuperclass().getDeclaredField("symbol");
            f1.setAccessible(true);
            f1.set(unit, symbolIfNone);
            f1.setAccessible(false);
        }

        return unit;
    }

    //...

    // usage
    getUnitWithNameSymbol(Units.GRAM, "Gram", "g"),

In my implementation, I keep a list of 'relevant' units that I keep these in that my app supports. You can find the full implementation here: https://github.com/Epic-Breakfast-Productions/OpenQuarterMaster/blob/main/software/libs/open-qm-core/src/main/java/com/ebp/openQuarterMaster/lib/core/UnitUtils.java