On 8/29/2012 4:20 PM, Stuart Marks wrote:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
436 switch (actions) {
437 case SecurityConstants.FILE_READ_ACTION:
438 return READ;
Oops, you have reverted the change to use switch-on-Strings by
webrev.03. Why?
A fair question. I had instigated some internal conversation about
reverting this change, so here's the explanation. (Good catch, by the
way.)
The original code was like this:
427 private static int getMask(String actions) {
...
435 // Check against use of constants (used heavily within
the JDK)
436 if (actions == SecurityConstants.FILE_READ_ACTION) {
437 return READ;
438 } else if (actions == SecurityConstants.FILE_WRITE_ACTION) {
The various SecurityConstants being used here are Strings.
Note that this is doing String comparisons using == which is usually a
bug. But this code is relying on String interning to provide object
identity for equal string literals, in order to provide a fast path
optimization for these common cases. Changing this code to use
switch-over-strings would be semantically equivalent, but it would
probably slow things down, since switch provides equals() semantics
instead of == semantics.
This would be a fine point to highlight in the comments around this check!
-Joe
