Long valueOf instead of new Long (original) (raw)

Pavel Rappo pavel.rappo at oracle.com
Mon Jun 30 10:11:25 UTC 2014

I've updated the webrev. It includes all the changes we've discussed so far plus these:

http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/macosx/classes/sun/font/CStrike.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/com/sun/tools/example/debug/tty/BreakpointSpec.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeQualifiedLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeQualifiedStaticLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/share/classes/sun/reflect/UnsafeStaticLongFieldAccessorImpl.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/solaris/classes/java/util/prefs/FileSystemPreferences.java.sdiff.html http://cr.openjdk.java.net/~prappo/8048267/webrev.03/src/solaris/classes/sun/awt/X11/XFocusProxyWindow.java.sdiff.html

Andrej, about the 0L -> Long0 change. I think it's not necessary since this case is maybe different from others. There's even a comment on it:

    // Should never happen... but stay safe all the same.
    //
    else return 0L;

Anyway at a runtime 0L and Long0 will be the same object. So don't worry about that.

I think we're almost ready to go.

Thanks, -Pavel

On 28 Jun 2014, at 00:09, Otávio Gonçalves de Santana <otaviojava at java.net> wrote:

I found more two unnecessary valueOf. About Andrej, is it not possible add two people in "Contributed-by:" tag?

diff -r d02b062bc827 src/share/classes/com/sun/tools/example/debug/tty/Commands.java --- a/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Fri Jun 13 11:21:30 2014 -0700 +++ b/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Fri Jun 27 20:06:28 2014 -0300 @@ -935,7 +935,7 @@ try { methodInfo = loc.sourceName() + MessageOutput.format("line number", - new Object [] {new Long(lineNumber)}); + new Object [] {lineNumber}); } catch (AbsentInformationException e) { methodInfo = MessageOutput.format("unknown"); } @@ -946,7 +946,7 @@ meth.declaringType().name(), meth.name(), methodInfo, - new Long(pc)}); + pc}); } else { MessageOutput.println("stack frame dump", new Object [] {new Integer(frameNumber + 1),

On Fri, Jun 27, 2014 at 5:16 PM, Andrej Golovnin <andrej.golovnin at gmail.com> wrote: Hi Pavel, I'm not sure what the style guide for the source code says, but there is a space between the cast operator and the field name in src/share/classes/com/sun/jmx/snmp/daemon/SnmpAdaptorServer.java (line 881): @Override public Long getSnmpOutGenErrs() { - return new Long(snmpOutGenErrs); + return (long) snmpOutGenErrs; } In all other changes there is no space after the cast operator. > Also, I removed one useless creation of a Long object here: > > (line 191): > > http://cr.openjdk.java.net/~prappo/8048267/webrev.01/src/share/classes/sun/security/krb5/internal/KerberosTime.java.sdiff.html > http://cr.openjdk.java.net/~prappo/8048267/webrev.02/src/share/classes/sun/security/krb5/internal/KerberosTime.java.sdiff.html And I have found one more :-) in KerberosTime.java: 252 public int getSeconds() { 253 Long templong = kerberosTime / 1000L; 254 return templong.intValue(); 255 } This can be changed to: 252 public int getSeconds() { 253 long templong = kerberosTime / 1000L; 254 return (int) templong; 255 } > > I wonder if we should leave a cast to int here: > > (line 383): > > http://cr.openjdk.java.net/~prappo/8048267/webrev.01/src/share/classes/sun/management/snmp/jvminstr/JvmMemoryImpl.java.sdiff.html > http://cr.openjdk.java.net/~prappo/8048267/webrev.02/src/share/classes/sun/management/snmp/jvminstr/JvmMemoryImpl.java.sdiff.html > > Well it's probably nothing to worry about, but strictly speaking this changes the behaviour. Before the change, long was truncated to fit int. And now it's not. I would not change the behavior now. I think it is better to file a new issue and change it in a separate commit. Having this change in a separate commit may simplify tracking this change back in case it would cause some problems (I don't believe it). And in the new issue you may provide better description for this change. And a minor comment for JvmMemoryImpl.java: Maybe it is better to use Long0 in the line 387. So the code of the method JvmMemoryImpl.getJvmMemoryPendingFinalCount() will look similar to the code of other methods in the JvmMemoryImpl class. They all use the Long0 constant to return 0L. In sun/management/snmp/jvminstr/JvmThreadingImpl.java in the line 313: 312 public Long getJvmThreadPeakCount() throws SnmpStatusException { 313 return (long)getThreadMXBean().getPeakThreadCount(); 314 } There is one space too much between "return" and the cast operator. The additional space was in the original version too, but maybe we can clean up here the code a little bit. > > P.S. Andrej, it looks like you don't have an 'Author' status. Well, that's a pity. We could mention you in the 'Reviewed-by' line. Your contributions are really good. Thanks! But I don't really care about it as long as I can help to improve the overall code quality. Best regards, Andrej Golovnin

-- Atenciosamente. Otávio Gonçalves de Santana blog: http://otaviosantana.blogspot.com.br/ twitter: http://twitter.com/otaviojava site: http://www.otaviojava.com.br (11) 98255-3513

More information about the core-libs-dev mailing list