Tim Ellison wrote:
On 08/Oct/2009 14:21, Oliver Deakin wrote:
Tim Ellison wrote:
Part of the patch for HARMONY-6346 includes some improvements to the way
we handle localized exception messages. That prompted me to write down
a few thoughts...
I don't really like the way we do I18N of exception messages. We create
message string like this
throw new IOException(Messages.getString("archive.1E"));
One way to handle this could be to use an exception manager class.
Rather than carrying out the lookup on the error message at the time the
exception is thrown, instead associate the exception object with the
error message key using a exception manager class. Then in the
getMessage(), getLocalizedMessage() etc. methods the exception will call
into the exception manager to get it's message (and can cache it locally
in case of future lookups) if one exists.
So instead of:
throw new IOException(Messages.getString("archive.1E"));
you might have (here create() returns the exception passed to it as the
first parameter):
throw ExceptionManager.create(new IOException(), "archive.1E");
Once the exception is caught and it's message string queried, it will
call ExceptionManager.getMessage(this) to see if it has an associated
message available. This way the error message lookup will only happen if
the message is actually queried. The references in the ExceptionManager
could be soft so that if the exception is not caught or is discarded,
then the ExceptionManager does not unnecesarily hang onto it's error
message.
Yep, so to replay what you just said, the 'guts' of the ExceptionManager
would be a WeakHashMap<Throwable, Object[]> of 'pending' exceptions that
may be asked for their formatted message.
The Object[] would contain the message id and arguments that only get
looked up and formatted if asked.
Sounds about right to me.
The string "archive.1E" is a key into the resource files loaded by the
Messages class based on the current locale.
1) I have no concrete proof, but I'm prepared to bet that 90% of the
exceptions thrown never get asked for their message string. Yet we are
setting it eagerly - mainly because there is no mechanism on exceptions
to answer the message lazily.
Simple messages may not be too bad, but we also have formatted messages
where we replace token in the message with actual parameters. A
particular bad example requires formatting more argument strings, e.g.
throw new IllegalArgumentException(
Msg.getString("K031e", new String[] { "query", uri.toString() }));
in this case we format the URI and format the message string, and
probably never use the answer.
For this example, the create() method could be made to accept variable
arguments. It's first argument would be the exception object, the second
would be the message identifier, and any subsequent arguments would be
the values to be inserted into tokens in the message string. This would
mean that in the above example the uri.toString() would still be
evaluated at the time the exception was created, but the lookup and
formatting of the error string would only be done when the actual
exception message was queried. (I havn't looked deeply at this case so
it may be more complex then that).
...and if you wanted to be sneaky, you could change the message id to be
the actual string for `you favorite locale` which would mean there was
no loading of message resource bundles if you happened to be running in
the most-favored-locale, e.g.
throw ExceptionManager.create(new IOException(), "Stream is closed");
True, although I'm not entirely convinced this is a good idea for a
couple of reasons:
- Changing all our message files to have "Full English message"="Full
translated message" might make them quite cluttered and hard to read.
- Some messages may contain the '=' character (for example, archive.15)
which would mess up the parsing of message files.
All this is dependent upon the premise that messages are not used often,
I need to check that first.
Indeed - this might be overkill if it only has a small impact.
Regards,
Oliver
Regards,
Tim
2) The use of ResourceBundles for handling the strings seems like
overkill too.
Each time we need a string, we load up the entire set of error messages
for that module into a Properties (i.e. hash table) and hold onto them
forever.
The HARMONY-6346 patch makes the resource file soft referenced so that
may help, but I can't help thinking there is a simpler mechanism for
loading the one or two strings that we need rather than the whole lot +
data structure each time.
Regards,
Tim
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
