关于java代码质量的问题

一、

错误码：WMI_WRONG_MAP_ITERATOR 
案例一： 

 

案例二： 

 


Bug: Method JTAMainFrame.initView(JFrame) makes inefficient use of keySet iterator instead of entrySet iterator 
Pattern id: WMI_WRONG_MAP_ITERATOR, type: WMI, category: PERFORMANCE 

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup. 


解释： 
很多人都这样遍历Map，没错，但是效率很低，先一个一个的把key遍历，然后在根据key去查找value，这不是多此一举么，为什么不遍历entry（桶）然后直接从entry得到value呢？它们的执行效率大概为1.5:1（有人实际测试过）。 
我们看看HashMap.get方法的源代码： 

Java代码  

1. 1.  public V get(Object key) {    

2. 2.      if (key == null)    

3. 3.          return getForNullKey();    

4. 4.      int hash = hash(key.hashCode());    

5. 5.      for (Entry<K,V> e = table[indexFor(hash, table.length)];    

6. 6.           e != null;    

7. 7.           e = e.next) {    

8. 8.          Object k;    

9. 9.          if (e.hash == hash && ((k = e.key) == key || key.equals(k)))    

10. 10.             return e.value;    

11. 11.     }    

12. 12.     return null;    

13. 13. }    

从这里可以看出查找value的原理，先计算出hashcode，然后散列表里取出entry，不管是计算hashcode，还是执行循环for以及执行equals方法，都是CPU密集运算，非常耗费CPU资源，如果对一个比较大的map进行遍历，会出现CPU迅速飚高的现象，直接影响机器的响应速度，在并发的情况下，简直就是一场灾难。 
解决方法： 

Java代码  

1. 1.  for (Map.Entry<String, JMenu> entry : menuList.entrySet()) {    

2. 2.      mb.add(entry.getValue());  

3. }  

 




错误码：EI_EXPOSE_REP2 

案例 

 

Bug: SingleNePollConfigDialog.collectValues(Hashtable) may expose internal representation by storing an externally mutable object into SingleNePollConfigDialog.values 
Pattern id: EI_EXPOSE_REP2, type: EI2, category: MALICIOUS_CODE 

This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations. 
翻译愿意： 
此代码存储到一个到对象的内部表示外部可变对象的引用。如果实例是由不受信任的代码，并以可变对象会危及安全或其他重要的属性选中更改访问，你需要做不同的东西。存储一个对象的副本，在许多情况下是更好的办法。 

解释： 
DO类实例产生之后，里面包含的Date不是原始数据类型，导致其gmtCrate属性不光DO实例的set方法可以改变其值，外部引用修改之后也可能导致gmtCreate 被改变，会引起可能的不安全或者错误。 
这个是一个不好的实践，不过我们应用里面DO都是比较简单使用，不太会出现这种情况。 

解决方法： 
修改成： 

Java代码  

1. public Date getGmtCreate() {  

2.         if(this.gmtCreate != null)  

3.            return new Date(this.gmtCreate.getTime()); //正确值  

4.         else   

5.             return null;  

6. }  

总结：这个其实是说可变类和不可变类的问题， 
可变类：当你获得这个类的一个实例引用时，你可以改变这个实例的内容。 
不可变类：当你获得这个类的一个实例引用时，你不可以改变这个实例的内容。不可变类 的实例一但创建，其内在成员变量的值就不能被修改 ； 
DO是一个可变类，但是最好是只提供set方法才能改变其实例的的成员变量的值，减少被修改的风险。 

• 

• 

• 

• 

• 

• 

• 二、

• 错误码：DM_FP_NUMBER_CTOR 
   
  
  Bug: Method OnlineLicenseDAOTest.testUpdateOnlineLicenseByOnlineMerchantId() invokes inefficient Double.valueOf(double) constructor; use OnlineLicenseDAOTest.java:[line 81] instead 
  Pattern id: DM_FP_NUMBER_CTOR, type: Bx, category: PERFORMANCE 
  Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster. 
  Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float. 
  解释： 
  采用new Ddouble(double)会产生一个新的对象，采用Ddouble.valueOf(double)在编译的时候可能通过缓存经常请求的值来显著提高空间和时间性能。 
  
  解决方法： 
  采用Ddouble.valueOf方法 
  
  类似的案例 
  
  
  
   
  
  类似的还有： 
  错误码：DM_NUMBER_CTOR 
  new Integer(int) 和 Integer.valueOf(int)  
  bug描述： 
  [Bx] Method invokes inefficient Number constructor; use static valueOf instead [DM_NUMBER_CTOR] 
  Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster. 
  说明: 
  [参考]http://www.cnblogs.com/hyddd/articles/1391318.html 
  FindBugs推荐使用Integer.ValueOf(int)代替new Integer(int)，因为这样可以提高性能。如果当你的int值介于-128～127时，Integer.ValueOf(int)的效率比Integer(int)快大约3.5倍。 
  下面看看JDK的源码，看看到Integer.ValueOf(int)里面做了什么优化： 
  

• Java代码  

• public static Integer valueOf(int i) {  

•   final int offset = 128;  

•   if (i >= -128 && i <= 127) { // must cache  

•     return IntegerCache.cache[i + offset];  

•    }  

•   return new Integer(i);  

• }   

•   

•     

•   

• private static class IntegerCache {  

•   private IntegerCache(){}  

•       

•   static final Integer cache[] = new Integer[-(-128) + 127 + 1];  

•   static {  

•   for(int i = 0; i < cache.length; i++)  

•       cache = new Integer(i - 128);  

•    }  

• }   

• 
  从源代码可以知道，ValueOf对-128～127这256个值做了缓存(IntegerCache)，如果int值的范围是：-128～127，在ValueOf(int)时，他会直接返回IntegerCache的缓存给你。 
  所以你会看到这样的一个现象： 
  

• Java代码  

• public static void main(String []args) {  

•       Integer a = 100;  

•       Integer b = 100;  

•       System.out.println(a==b);  

•   

•       Integer c = new Integer(100);  

•       Integer d = new Integer(100);  

•       System.out.println(c==d);  

• }  

• 
  结果是： 
  true 
  false 
  因为：java在编译的时候 Integer a = 100; 被翻译成-> Integer a = Integer.valueOf(100);，所以a和b得到都是一个Cache对象，并且是同一个！而c和d是新创建的两个不同的对象，所以c自然不等于d。 
  再看看这段代码： 
  

• Java代码  

• public static void main(String args[]) throws Exception{  

•          Integer a = 100;  

•          Integer b = a;  

•          a = a + 1;　　//或者a++;  

•          System.out.println(a==b);  

• }  

• 
  结果是：false 
  因为在对a操作时(a=a+1或者a++)，a重新创建了一个对象，而b对应的还是缓存里的100，所以输出的结果为false。 
  
  
  
  错误码：DM_BOOLEAN_CTOR 
   
  
  Bug: Bad attempt to compute absolute value of signed 32-bit hashcode 
  Pattern id: RV_ABSOLUTE_VALUE_OF_HASHCODE, type: RV, category: CORRECTNESS 
  This code generates a hashcode and then computes the absolute value of that hashcode. If the hashcode is Integer.MIN_VALUE, then the result will be negative as well (since Math.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE). 
  One out of 2^32 strings have a hashCode of Integer.MIN_VALUE, including "polygenelubricants" "GydZG_" and ""DESIGNING WORKHOUSES". 
  解释： 
  此代码产生的哈希码，然后计算该哈希码的绝对值。如果哈希码是Integer.MIN_VALUE的，那么结果将是负面的，以及（因为Math.abs（Integer.MIN_VALUE的）==Integer.MIN_VALUE的）。 
  
  解决方法： 
  在使用之前判断一下是否是为Integer.MIN_VALUE 
  
  

• Java代码  

• int iTemp = sellerId.hashCode();  

•        if(iTemp != Integer.MIN_VALUE) {  

•          number = Math.abs(iTemp) % 12;  

• } else {  

•        number = Integer.MIN_VALUE % 12;  

• }  

• 
  
  

• 

  
  

• 

• 

• 三、

• 错误码：SE_NO_SERIALVERSIONID 
  
  
   
  
  
  Bug: WindowHandlerManager$MySingleSelectionModel is Serializable; consider declaring a serialVersionUID 
  Pattern id: SE_NO_SERIALVERSIONID, type: SnVI, category: BAD_PRACTICE 
  This class implements the Serializable interface, but does not define a serialVersionUID field.  A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID. 
  
  解释： 
  实现了Serializable接口，却没有实现定义serialVersionUID字段，序列化的时候，我们的对象都保存为硬盘上的一个文件，当通过网络传输或者其他类加载方式还原为一个对象时，serialVersionUID字段会保证这个对象的兼容性，考虑两种情况： 
  1. 新软件读取老文件，如果新软件有新的数据定义，那么它们必然会丢失。 
  2. 老软件读取新文件，只要数据是向下兼容的，就没有任何问题。 
  序列化会把所有与你要序列化对象相关的引用（包括父类，特别是内部类持有对外部类的引用，这里的例子就符合这种情况）都输出到一个文件中，这也是为什么能够使用序列化能进行深拷贝。这种序列化算法给我们的忠告是，不要把一些你无法确定其基本数据类型的对象引用作为你序列化的字段（比如JFrame），否则序列化后的文件超大，而且会出现意想不到的异常。 
  解决方法： 
  定义serialVersionUID字段 
  
  
  错误码：DM_GC 
  
   
  
  bug: DBExportTask2.exportDBRecords(DBExportProperty, String) forces garbage collection; extremely dubious except in benchmarking code 
  Pattern id: DM_GC, type: Dm, category: PERFORMANCE 
  解释： 
  有两点： 
  1. System.gc()只是建议，不是命令，JVM不能保证立刻执行垃圾回收。 
  2. System.gc()被显示调用时，很大可能会触发Full GC。 
  GC有两种类型：Scavenge GC和Full GC，Scavenge GC一般是针对年轻代区（Eden区）进行GC，不会影响老年代和永生代（PerGen），由于大部分对象都是从Eden区开始的，所以Scavenge GC会频繁进行，GC算法速度也更快，效率更高。但是Full GC不同，Full GC是对整个堆进行整理，包括Young、Tenured和Perm，所以比Scavenge GC要慢，因此应该尽可能减少Full GC的次数。 
  解决方法： 
  去掉System.gc() 
  
  
  
  错误码：DP_DO_INSIDE_DO_PRIVILEGED 
  
   
  
  Bug: com.taobao.sellerservice.core.test.BaseTestJunit.autoSetBean() invokes reflect.Field.setAccessible(boolean), which should be invoked from within a doPrivileged block 
  Pattern id: DP_DO_INSIDE_DO_PRIVILEGED, type: DP, category: BAD_PRACTICE 
  This code invokes a method that requires a security permission check. If this code will be granted security permissions, but might be invoked by code that does not have security permissions, then the invocation needs to occur inside a doPrivileged block. 
  此代码调用一个方法，需要一个安全权限检查。如果此代码将被授予安全权限，但可能是由代码不具有安全权限调用，则需要调用发生在一个doPrivileged的块。 
  解决方法：一般情况下，我们并不能对类的私有字段进行操作，利用反射也不例外，但有的时候，例如要序列化的时候，我们又必须有能力去处理这些字段，这时候，我们就需要调用AccessibleObject上的setAccessible()方法来允许这种访问，而由于反射类中的Field，Method和Constructor继承自AccessibleObject，因此，通过在这些类上调用setAccessible()方法，我们可以实现对这些字段的操作。但有的时候这将会成为一个安全隐患，为此，我们可以启用java.security.manager来判断程序是否具有调用setAccessible()的权限。默认情况下，内核API和扩展目录的代码具有该权限，而类路径或通过URLClassLoader加载的应用程序不拥有此权限。例如：当我们以这种方式来执行上述程序时将会抛出异常 
  增加：} catch (SecurityException e) { 
  
  
  错误码：NP_NULL_ON_SOME_PATH 
  
   
  
  Bug: Possible null pointer dereference of busCatId 
  Pattern id: NP_NULL_ON_SOME_PATH, type: NP, category: CORRECTNESS 
  There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed. Of course, the problem might be that the branch or statement is infeasible and that the null pointer exception can't ever be executed; deciding that is beyond the ability of FindBugs. 
  解释： 
  方法中存在空指针 
  解决方法： 
  增加字段busCatId为空的判断 

• 

  
  

• 

• 
  

• 

• 
  

• 

• 四、
  

错误码：RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 
  

 

Bug: Redundant nullcheck of bean1, which is known to be non-null 
Pattern id: RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE, type: RCN, category: STYLE 
This method contains a redundant check of a known non-null value against the constant null. 
解释：这种方法包含了一个称为非空对空值的不断重复检查。 
修改为： 


 



错误码：SS_SHOULD_BE_STATIC 


 

Bug: Unread field: ADDRESS_KEY; should this field be static? 
Pattern id: SS_SHOULD_BE_STATIC, type: SS, category: PERFORMANCE 
This class contains an instance final field that is initialized to a compile-time static value. Consider making the field static. 
解释： 
final成员变量表示常量，只能被赋值一次，赋值后值不再改变。 
这个类包含的一个final变量初始化为编译时静态值。考虑变成静态常量 
解决方法： 
增加static关键字 



错误码：NM_METHOD_NAMING_CONVENTION 

 

Bug: The method name MsmPlanDAOTest.TestViewMsmPlanList() doesn't start with a lower case letter 
Pattern id: NM_METHOD_NAMING_CONVENTION, type: Nm, category: BAD_PRACTICE 
Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized. 
解释： 
方法应该是动词，与第一个字母小写混合的情况下，与每个单词的首字母大写的内部。 
解决方法： 
方法名称小写就通过了。我们写代码还是要规范啊，虽然是单元测试的代码！ 

• 

• 

• 

• 

• 五、

• REC_CATCH_EXCEPTION 
  
  
  
  
  Bug: Exception is caught when Exception is not thrown 
  Pattern id: REC_CATCH_EXCEPTION, type: REC, category: STYLE 
  
  This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs. 
  
  这想写会无意中把RuntimeException也捕获了，有可能导致潜在的bug。 JVM对RuntimeException有统一的捕获机制，让JVM来处理它。 
  
  在try/catch块中捕获异常，但是异常没有在try语句中抛出而RuntimeException又没有明确的被捕获 
  
  1.比较推荐的写法一般如下： 
  
  2.捕获了异常，一定要处理异常 
  还有人在catch里面什么都不写，就写上 
  
  3.避免在大的语句块里面写try,catch，因为本身也比较耗费时间，而且不便于调试和发现问题。 
  
  
  
  
  Bug: Call to equals() comparing different types 
  Pattern id: EC_UNRELATED_TYPES, type: EC, category: CORRECTNESS 
  
  
  
  
  This method calls equals(Object) on two references of different class types with no common subclasses. Therefore, the objects being compared are unlikely to be members of the same class at runtime (unless some application classes were not analyzed, or dynamic class loading can occur at runtime). According to the contract of equals(), objects of different classes should always compare as unequal; therefore, according to the contract defined by java.lang.Object.equals(Object), the result of this comparison will always be false at runtime. 
  
  两个不同类型的对象调用equals，将永远返回false，除非你重写了equals方法。 
  
  调用equals方法比较不同类型的类 
  
  This method uses using pointer equality to compare two references that seem to be of different types. The result of this comparison will always be false at runtime. 

• Java代码  

• try {  

• }catch(IOException e){  

•   

• }finally{  

• }  

• 

•