apfloat icon indicating copy to clipboard operation
apfloat copied to clipboard

For ApfloatRuntimeException based exceptions can we please have a Localizable interface to define all the exception messages?

Open axkr opened this issue 1 year ago • 2 comments

For ApfloatRuntimeException based exceptions can we please have a Localizable interface to define all the exception messages?

See the example in the Hipparchus repo:

  • https://github.com/Hipparchus-Math/hipparchus/blob/master/hipparchus-core/src/main/java/org/hipparchus/exception/LocalizedCoreFormats.java

By comparing the getSpecifier() return value, the caller can better determine, whats the root cause, instead of comparing the message string.

See also:

  • https://github.com/Hipparchus-Math/hipparchus/blob/ae014acc583fc85cbbcac510d67a3557705f5eed/hipparchus-core/src/main/java/org/hipparchus/exception/MathRuntimeException.java#L39
  • https://github.com/Hipparchus-Math/hipparchus/blob/ae014acc583fc85cbbcac510d67a3557705f5eed/hipparchus-core/src/main/java/org/hipparchus/exception/MathRuntimeException.java#L101

axkr avatar Jun 21 '24 15:06 axkr

If you want localized messages for exceptions, then I think those could be added, by overriding Throwable.getLocalizedMessage() and adding some logic that takes the localized message from a resource bundle.

However if your intention is to have an Enum that enumerates all possible messages for ApfloatRuntimeException (so that there could not be any other messages than those specified in that Enum) then I think the situation is more problematic. I never thought that should be the case, and I don't know if somebody has already implemented in some other project a line of code that instantiates an ApfloatRuntimeException with whatever message, so any such code should remain backwards compatible.

A resource bundle based implementation would anyways need a key for every localized message (for the resource bundle, to get the actual localized message) so perhaps this key could be added to the exception, but it would be a String, not an Enum, and it could also be null for any "legacy" exception that was constructed just from a message, and not a message key.

Something like:

public class ApfloatRuntimeException
    extends RuntimeException
{
    public ApfloatRuntimeException(String message, Throwable cause, String key, String... args)
    {
        super(message, cause);
        this.key = key;
        this.args = args.clone();
    }

    @Override
    public String getLocalizedMessage()
    {
        String message;
        try
        {
            String pattern = ResourceBundle.getBundle("org.apfloat.apfloat-errors").getString(this.key);
            message = MessageFormat.format(pattern, this.args);
        }
        catch (MissingResourceException mre)
        {
            message = super.getLocalizedMessage();
        }
        return message;
    }

    public String getKey()
    {
        return this.key;
    }
    // ...
}

mtommila avatar Jun 24 '24 18:06 mtommila

I think Strings for most common exceptions are a good compromise.

axkr avatar Jun 29 '24 09:06 axkr

Implementation note: in addition to the new functionality in ApfloatRuntimeException there is a new subclass of ArithmeticException named ApfloatArithmeticException, which has the same implementation of getLocalizedMessage(). Both of these classes now implement a new interface named ApfloatLocalizedException.

There are some other exceptions that are thrown from various methods in the library, such as IllegalArgumentException. However probably no other exceptions than the two above are thrown as a result of a computation, so there would likely be no interest in creating a subclass like ApfloatIllegalArgumentException that would also implement this new interface.

mtommila avatar Sep 09 '24 20:09 mtommila

I have deployed a snapshot.

mtommila avatar Sep 09 '24 21:09 mtommila