On 1/24/17 12:38 PM, Adam Petcher wrote:
Thanks Sean and Mandy for the feedback. I believe I incorporated all
feedback into the latest patch, with one exception (Sean see below).
http://cr.openjdk.java.net/~apetcher/8168075/webrev.02/
In PolicyParser.ParsingException, there is a comment stating that the
MessageFormat is stored in the exception, and it is only formatted when
getLocalizedMessage() is called on the exception. This arrangement
avoids unnecessary formatting and permission checks in cases where the
message is never localized and formatted. I kept this arrangement after
I replaced MessageFormat with LocalizedMessage. If there is no value to
delaying/avoiding the message formatting, then I can remove the
constructor and non-static method as you suggest.
Ok I see. I think now you better leave that code as is since it was
added as part of JDK-8150468.
Here are the changes I made based on this feedback:
1) I changed the name of LocalizedMessage.toLocalizedString to "format"
to make it more clear that I was trying to mirror the behavior of
MessageFormat (and also to shorten the name and remove redundancy).
2) I added a comment to the constructor describing how it can be used to
delay/avoid message formatting.
3) I changed all references to LocalizedMessage that don't involve
ParsingException to use only the static method of LocalizedMessage, as
you suggested.
It looks like there is a leftover import for PolicyUtil in
PolicyParser.java. Otherwise looks good and I can push this fix for you
after you send me an updated patch/webrev.
--Sean
