admin@glassfish.java.net

Re: Code Review Needed

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Tue, 13 Feb 2007 11:21:48 -0800

I dunno. I compiled a class with a comment in a catch, versus an empty
catch block.
The class files were exactly the same length but had 2 one-char
differences.
Adding comments in the regular code created classes with no differences.

Maybe the compiler does note the difference in the class files for a
debug compile?
This is fun R&D but I've run out of time. Your turn!


kedar wrote:

> Looks mostly good. I am not too sure, but if you have
> improved the logic at places, please make sure that
> you run appropriate tests beyond QuickLook.
>
> Other thing I observed that FindBugs complains
> about the following:
> try {
> foo.bar(); //may throw exception
> } catch(Exception e) {}
>
> but not:
>
> try {
> foo.bar(); //may throw exception
> } catch(Exception e) {
> // purposely squelched
> }
>
> Does it do a source-level check?
>
> Thanks,
> Kedar
>
> Byron Nevins wrote:
>
>> Issue 2183
>>
>> FindBugs issues resolved. Somebody please review the changes by EOB
>> Tuesday 2/13/07
>>
>> -- the diffs are attached to the issue and to this email.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index:
>> mbeans/src/java/com/sun/enterprise/ee/admin/dottedname/DottedNameClusterInfoImpl.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin-ee/mbeans/src/java/com/sun/enterprise/ee/admin/dottedname/DottedNameClusterInfoImpl.java,v
>>
>> retrieving revision 1.1.1.1
>> diff -u -r1.1.1.1 DottedNameClusterInfoImpl.java
>> ---
>> mbeans/src/java/com/sun/enterprise/ee/admin/dottedname/DottedNameClusterInfoImpl.java
>> 8 Aug 2006 19:48:40 -0000 1.1.1.1
>> +++
>> mbeans/src/java/com/sun/enterprise/ee/admin/dottedname/DottedNameClusterInfoImpl.java
>> 13 Feb 2007 01:57:05 -0000
>> @@ -269,18 +269,24 @@
>> }
>>
>> public String[] getAllResourceNames() {
>> - final Set resNames = Collections.EMPTY_SET;
>> + HashSet<String> resNames = new HashSet<String>();
>> + try {
>> String[] res;
>> final Iterator i= getAllTargets().iterator();
>> while(i.hasNext()){
>> res =
>> getResourcesProxy().listResourceReferencesAsString((String)i.next());
>> - resNames.add(ArrayConversion.toSet(res));
>> + + if(res == null)
>> + continue;
>> + + for(String s : res)
>> + resNames.add(s);
>> }
>> } catch (Exception e) {
>> DottedNameLogger.logException(e);
>> }
>> - return (String[])resNames.toArray();
>> + return (String[])resNames.toArray(new String[resNames.size()]);
>> }
>>
>> private Collection getAllTargets() throws
>> DottedNameServerInfo.UnavailableException,
>> MalformedObjectNameException {
>> @@ -310,18 +316,23 @@
>>
>>
>> public String[] getAllApplicationNames() throws
>> MalformedObjectNameException {
>> - final Set appsNames = Collections.EMPTY_SET;
>> - try {
>> + Set<String> appsNames = new HashSet<String>();
>> + try {
>> String[] apps;
>> final Iterator i= getAllTargets().iterator();
>> while(i.hasNext()){
>> apps =
>> getApplicationsProxy().listApplicationReferencesAsString((String)i.next());
>>
>> - appsNames.add(ArrayConversion.toSet(apps));
>> +
>> + if(apps == null)
>> + continue;
>> + + for(String s : apps)
>> + appsNames.add(s);
>> }
>> - } catch (Exception e) {
>> + } catch (Exception e) {
>> DottedNameLogger.logException(e);
>> }
>> - return (String[])appsNames.toArray();
>> + return (String[])appsNames.toArray(new
>> String[appsNames.size()]);
>> }
>>
>> private MyResources getResourcesProxy() throws
>> MalformedObjectNameException {
>>
>> Index:
>> mbeanapi-impl/src/java/com/sun/enterprise/management/offline/AuthRealmConfigBeanHelper.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/enterprise/management/offline/AuthRealmConfigBeanHelper.java,v
>>
>> retrieving revision 1.2
>> diff -u -r1.2 AuthRealmConfigBeanHelper.java
>> ---
>> mbeanapi-impl/src/java/com/sun/enterprise/management/offline/AuthRealmConfigBeanHelper.java
>> 25 Dec 2005 03:40:30 -0000 1.2
>> +++
>> mbeanapi-impl/src/java/com/sun/enterprise/management/offline/AuthRealmConfigBeanHelper.java
>> 13 Feb 2007 01:58:31 -0000
>> @@ -171,7 +171,13 @@
>> String[] types )
>> {
>> Object result = null;
>> - final int numArgs = args == null ? 0 : args.length;
>> + + if(args == null)
>> + {
>> + unsupportedOperation( operationName, args, types );
>> + }
>> +
>> + final int numArgs = args.length;
>> if ( isStdFileRealm() &&
>> SUPPORTED_OPERATIONS.contains( operationName ) &&
>> numArgs >= 1 )
>> Index:
>> mbeanapi-impl/src/java/com/sun/enterprise/management/support/TestDummy.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeanapi-impl/src/java/com/sun/enterprise/management/support/TestDummy.java,v
>>
>> retrieving revision 1.5
>> diff -u -r1.5 TestDummy.java
>> ---
>> mbeanapi-impl/src/java/com/sun/enterprise/management/support/TestDummy.java
>> 17 Mar 2006 03:34:21 -0000 1.5
>> +++
>> mbeanapi-impl/src/java/com/sun/enterprise/management/support/TestDummy.java
>> 13 Feb 2007 01:58:31 -0000
>> @@ -235,9 +235,13 @@
>> final String[] signature )
>> {
>> Object result = null;
>> - final int numArgs = args == null ? 0 : args.length;
>> - if ( "addAttribute".equals( methodName ) && numArgs
>> == 2 )
>> + if(args == null)
>> + throw new RuntimeException("internal Error -- no args");
>> +
>> + final int numArgs = args.length;
>> + + if ( "addAttribute".equals( methodName ) && numArgs
>> == 2 )
>> {
>> addAttribute( (String)args[ 0 ], (Object)args[ 1 ] );
>> }
>> Index:
>> mbeans/src/java/com/sun/enterprise/admin/dottedname/DottedName.java
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeans/src/java/com/sun/enterprise/admin/dottedname/DottedName.java,v
>>
>> retrieving revision 1.4
>> diff -u -r1.4 DottedName.java
>> ---
>> mbeans/src/java/com/sun/enterprise/admin/dottedname/DottedName.java
>> 25 Dec 2005 03:42:01 -0000 1.4
>> +++
>> mbeans/src/java/com/sun/enterprise/admin/dottedname/DottedName.java
>> 13 Feb 2007 01:58:32 -0000
>> @@ -120,6 +120,18 @@
>> }
>> }
>>
>> + public int
>> + hashCode()
>> + {
>> + int hashcode = 17;
>> + hashcode = 37 * hashcode + mDomain.hashCode();
>> + hashcode = 37 * hashcode + mScope.hashCode();
>> + hashcode = 37 * hashcode + mParts.hashCode();
>> + + return hashcode;
>> + + }
>> + public boolean
>> equals( Object other )
>> {
>> @@ -447,7 +459,17 @@
>> }
>>
>>
>> - public boolean
>> + public int
>> + hashCode()
>> + {
>> + int hashcode = 17;
>> + hashcode = 37 * hashcode + mSourceString.hashCode();
>> + hashcode = 37 * hashcode + mParsed.hashCode();
>> + + return hashcode;
>> + }
>> + + public boolean
>> equals( Object other )
>> {
>> boolean equals = false;
>> Index:
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ApplicationsConfigMBean.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeans/src/java/com/sun/enterprise/admin/mbeans/ApplicationsConfigMBean.java,v
>>
>> retrieving revision 1.25
>> diff -u -r1.25 ApplicationsConfigMBean.java
>> ---
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ApplicationsConfigMBean.java
>> 19 Jan 2007 16:01:31 -0000 1.25
>> +++
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ApplicationsConfigMBean.java
>> 13 Feb 2007 01:58:34 -0000
>> @@ -181,7 +181,7 @@
>> protected static final String[] emptySignature = new String[]{};
>> - protected static StringManager localStrings =
>> + protected static final StringManager localStrings =
>> StringManager.getManager( ApplicationsConfigMBean.class );
>> @@ -603,10 +603,7 @@
>>
>> req.setExternallyManagedPath(dProps.getExternallyManaged());
>> DeploymentServiceUtils.setResourceOptionsInRequest(req,
>> dProps);
>> - if(props == null)
>> - props = new Properties();
>> - else
>> - props = dProps.prune();
>> + props = dProps.prune();
>>
>> req.addOptionalArguments(props);
>> Index:
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ConnectorConnectionPoolMBean.java
>>
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeans/src/java/com/sun/enterprise/admin/mbeans/ConnectorConnectionPoolMBean.java,v
>>
>> retrieving revision 1.4
>> diff -u -r1.4 ConnectorConnectionPoolMBean.java
>> ---
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ConnectorConnectionPoolMBean.java
>> 9 Nov 2006 22:16:25 -0000 1.4
>> +++
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ConnectorConnectionPoolMBean.java
>> 13 Feb 2007 01:58:35 -0000
>> @@ -569,7 +569,6 @@
>> String tgtName) throws MBeanException{
>> ObjectName mbean = null;
>> - StringBuffer strf = new StringBuffer();
>> boolean verbose = false;
>> ArrayList list = new ArrayList();
>>
>> Index:
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ResourcesMBean.java
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeans/src/java/com/sun/enterprise/admin/mbeans/ResourcesMBean.java,v
>>
>> retrieving revision 1.17
>> diff -u -r1.17 ResourcesMBean.java
>> ---
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ResourcesMBean.java
>> 1 Dec 2006 11:37:21 -0000 1.17
>> +++
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/ResourcesMBean.java
>> 13 Feb 2007 01:58:36 -0000
>> @@ -1224,7 +1224,9 @@
>> ObjectName CCPObjName = null;
>> try {
>> CCPObjName =
>> (ObjectName)getMBeanServer().invoke(o,
>> "getConnectorConnectionPoolByName", new Object[]{poolName}, new
>> String[]{"java.lang.String"});
>> - } catch (Exception ee){};
>> + } catch (Exception ee){
>> + // ignore this Exception
>> + }
>> if (CCPObjName != null) {
>> String cdn =
>> (String)getMBeanServer().getAttribute(CCPObjName,
>> "connection_definition_name");
>> if ((cdn != null) &&
>> ((cdn.equals(resType)) || ((cdn.equals(resType))))) {
>> @@ -1298,7 +1300,9 @@
>> ObjectName rac = null;
>> try {
>> rac = (ObjectName)
>> super.invoke("getResourceAdapterConfigByResourceAdapterName", new
>> Object[]{resAdapterConfig}, new String[]{"java.lang.String"});
>> - } catch (Exception ee){};
>> + } catch (Exception ee){
>> + // fall through -- the null test below will
>> handle the error
>> + };
>> if (rac == null) {
>> throw new
>> Exception(localStrings.getString("admin.mbeans.rmb.null_rac"));
>> }
>> Index:
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/SystemServicesMBean.java
>> ===================================================================
>> RCS file:
>> /cvs/glassfish/admin/mbeans/src/java/com/sun/enterprise/admin/mbeans/SystemServicesMBean.java,v
>>
>> retrieving revision 1.6
>> diff -u -r1.6 SystemServicesMBean.java
>> ---
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/SystemServicesMBean.java
>> 9 May 2006 21:53:37 -0000 1.6
>> +++
>> mbeans/src/java/com/sun/enterprise/admin/mbeans/SystemServicesMBean.java
>> 13 Feb 2007 01:58:37 -0000
>> @@ -261,11 +261,14 @@
>> }
>> downloadInfo.downloadFile = downloadFile;
>> long size = downloadInfo.downloadFile.length();
>> - downloadInfo.numChunks =
>> Math.round(size/ByteChunk.kChunkMaxSize);
>> - if (downloadInfo.numChunks * ByteChunk.kChunkMaxSize < size)
>> - {
>> - downloadInfo.numChunks += 1;
>> - }
>> + long chunkSize = (long)ByteChunk.kChunkMaxSize;
>> + long chunks = size / chunkSize;
>> + long leftovers = size % chunkSize;
>> + + if(leftovers > 0)
>> + chunks += 1;
>> + + downloadInfo.numChunks = (int)chunks;
>> /*
>> Debug.println("File=" +
>> downloadInfo.downloadFile.getAbsolutePath() +
>> ", " + "size=" + size + ", " +
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>