Review eleven Common Errors in Java code

Code reviews are one of the most important ways to eliminate bugs, which are especially effective in most cases. Because the code review itself targets objects, it is to overlook the problems and bugs of the entire code in the test process. In addition, code review is of great benefit to eliminate specific errors, especially those that can be easily found when reading the code, which are often not easily identified by tests on the machine. This article provides some constructive suggestions on common issues in Java code so that you can notice these common detailed errors during code review.

It is usually easier to pick a wrong job for others than to find your own mistakes. The existence of different perspectives also explains why the author needs to edit and why athletes need coaches. Not only should we not reject criticism from others, but we should welcome others to discover and point out the shortcomings in our programming work. We will benefit a lot. Code inspection is one of the most powerful technologies to improve code quality, code Review-errors found by colleagues looking for errors in the code-are different from those found in the test, so the relationship between the two is complementary, rather than competitive.

If the examiner can consciously find specific errors, rather than simply browsing the code without any purpose, the code review effect will get twice the result with half the effort. In this article, I listed 11 Common Errors in Java programming. You can add these errors to the checklist of your code review, so that after the code review, you can be sure that there are no such errors in your code.

　　I. common error 1 #: copying strings multiple times

One error that cannot be found during the test is to generate multiple copies of an immutable object. Immutable objects cannot be changed, so you do not need to copy them. The most common immutable object is string.

If you must change the content of a string object, you should use stringbuffer. The following code works properly:

String S = new string ("text here ");

However, this code has poor performance and does not need to be so complicated. You can also rewrite the above Code in the following ways:

String temp = "text here "; String S = new string (temp );

However, this code contains extra strings, which is not completely necessary. Better code:

String S = "text here ";

　　2. Common Error 2 #: no object returned by clone

Encapsulation is an important concept of object-oriented programming. Unfortunately, Java provides convenience for accidentally breaking the encapsulation-Java allows the return of reference for private data ). The following code reveals this point:

Import java. AWT. dimension; /*** Example class. The X and Y values shoshould never * be negative .*/ Public class example { Private dimension D = new dimension (0, 0 ); Public example (){} /*** Set height and width. Both height and width must be nonnegative * or an exception is thrown .*/ Public synchronized void setvalues (INT height, int width) throws illegalargumentexception { If (height <0 | width <0) Throw new illegalargumentexception (); D. Height = height; D. width = width; } Public synchronized dimension getvalues (){ // Ooops! Breaks Encapsulation Return D; } }

The example class ensures that the height and width values stored by the class are never negative. An exception is triggered when you try to use the setvalues () method to set a negative value. Unfortunately, because getvalues () returns a reference to D instead of a copy of D, you can write the following Destructive code:

Example EX = new example (); Dimension D = ex. getvalues (); D. Height =-5; D. width =-10;

Now, the example object has a negative value! If the getvalues () caller never sets the width and height values of the returned dimension object, such errors cannot be detected by testing alone.

Unfortunately, with the passage of time, the client code may change the value of the returned dimension object. At this time, it is boring and time-consuming to find the root cause of the error, especially in multi-threaded environments.

A better way is to get getvalues () to return a copy:

Public synchronized dimension getvalues (){ Return new dimension (D. x, D. y ); }

Now, the internal status of the example object is secure. The caller can change the copy status as needed. To modify the internal status of the example object, you must use setvalues.

　　Iii. Common Errors 3 # unnecessary cloning

We now know that the get method should return a copy of the internal data object instead of a reference. However, things are not absolute:

/*** Example class. The value shoshould never * be negative .*/ Public class example { Private integer I = new INTEGER (0 ); Public example (){} /*** Set x. x must be nonnegative * or an exception will be thrown */ Public synchronized void setvalues (int x) throws illegalargumentexception { If (x <0) Throw new illegalargumentexception (); I = new INTEGER (X ); } Public synchronized integer getvalue (){ // We Can't clone integers so we makea copy this way. Return new INTEGER (I. intvalue ()); } }

This code is safe, but as in error 1 #, it does extra work. An integer object, like a string object, is immutable once created. Therefore, it is safe to return an internal integer object instead of copying it.

The getvalue () method should be written as follows:

Public synchronized integer getvalue (){ // 'I' is immutable, so it is safe to return it instead of a copy. Return I; }

Java programs include more immutable objects than C ++ programs. Several unchangeable classes provided by JDK include:

· Boolean
· Byte
· Character
· Class
· Double
· Float
· Integer
· Long
· Short
· String
· Subclass of most exceptions

　　Iv. Common Errors 4 #: compile code to copy Arrays

Java allows you to clone arrays, but developers usually mistakenly write the following code. The problem is that three rows are used in the following loop. If the object clone method is used, one row can be used:

Public class example { Private int [] copy; /*** Save a copy of 'data'. 'data' cannot be null .*/ Public void savecopy (INT [] data ){ Copy = new int [data. Length]; For (INT I = 0; I <copy. length; ++ I) Copy [I] = data [I]; } }

This code is correct, but not complex. A better implementation of savecopy () is:

Void savecopy (INT [] data ){ Try { Copy = (INT []) data. Clone (); } Catch (clonenotsupportedexception e ){ // Can't get here. } }

If you clone arrays frequently, it would be a good idea to write the following tool method:

Static int [] clonearray (INT [] data ){ Try { Return (INT []) data. Clone (); } Catch (clonenotsupportedexception e ){ // Can't get here. } }

In this case, our savecopy looks more concise:

Void savecopy (INT [] data ){ Copy = clonearray (data ); }

5. Common Errors 5 # copy the wrong data

Sometimes programmers know that they must return a copy, but accidentally copy the wrong data. The following code deviates from the programmer's intention because only part of the data copy work is done:

Import java. AWT. dimension; /*** Example class. The height and width values shocould never * be Negative .*/ Public class example { Static final public int total_values = 10; Private dimension [] d = new dimension [total_values]; Public example (){} /*** Set height and width. Both height and width must be nonnegative * or an exception will be thrown .*/ Public synchronized void setvalues (INT index, int height, int width) throws illegalargumentexception { If (height <0 | width <0) Throw new illegalargumentexception (); If (d [Index] = NULL) D [Index] = new dimension (); D [Index]. Height = height; D [Index]. width = width; } Public synchronized dimension [] getvalues () Throws clonenotsupportedexception { Return (dimension []) D. Clone (); } }

The problem here is that the getvalues () method only clones the array, but does not clone the dimension object contained in the array. Therefore, although the caller cannot change the internal array to direct its elements to different dimension objects, the caller can change the content of the internal array elements (that is, the dimension object. A better version of the getvalues () method is:

Public synchronized dimension [] getvalues () throws clonenotsupportedexception { Dimension [] Copy = (dimension []) D. Clone (); For (INT I = 0; I <copy. length; ++ I ){ // Note: dimension isn' t cloneable. If (D! = NULL) Copy [I] = new dimension (d [I]. Height, d [I]. width ); } Return copy; }

When cloning multi-dimensional arrays of atomic data, a similar error occurs. The atomic types include int and float. It is correct to simply clone an int-type one-dimensional array, as shown below:

Public void store (INT [] data) throws clonenotsupportedexception { This. Data = (INT []) data. Clone (); // OK }

Copying a two-dimensional array of the int type is more complicated. Java does not have a two-dimensional array of the int type. Therefore, a two-dimensional array of the int type is actually a one-dimensional array of the int type []. A simple clone of the int [] [] type array will make the same error as the first version of the getvalues () method in the preceding example. Therefore, avoid doing this. The following example demonstrates the incorrect and correct practices when cloning an int-type two-dimensional array:

Public void wrongstore (INT [] [] data) throws clonenotsupportedexception { This. Data = (INT [] []) data. Clone (); // not OK! } Public void rightstore (INT [] [] data ){ // OK! This. Data = (INT [] []) data. Clone (); For (INT I = 0; I <data. length; ++ I ){ If (Data! = NULL) This. Data [I] = (INT []) data [I]. Clone (); } }

　　 6. Common Errors 6 #: Check whether the result of the new operation is null

A beginner in Java programming sometimes checks whether the result of the new operation is null. Possible check code:

Integer I = new INTEGER (400 ); If (I = NULL) Throw new nullpointerexception ();

Check is of course not wrong, but not necessary. The IF and throw lines of code are completely wasted. Their only function is to make the whole program more bloated and run slowly.

C/C ++ programmers often do this when writing Java programs, because it is necessary to check the returned results of malloc () in C, otherwise, an error may occur. Check whether the result of the new operation in C ++ may be a good programming behavior, depending on whether the exception is enabled (many compilers allow exceptions to be disabled, in this case, if the new operation fails, null is returned ). In Java, the new operation does not allow the return of null. If the return of null is true, it is likely that the virtual machine crashes. In this case, even if the returned results are checked, it will not help.

　　7. Common Errors 7 #: replace. equals with =

In Java, there are two ways to check whether two data items are equal: Use the = operator, or use the. Equals method implemented by all objects. The atomic type (INT, flosat, Char, etc.) is not an object, so they can only use the = Operator, as shown below:

Int x = 4; Int y = 5; If (x = y) System. Out. println ("hi "); // This 'if 'test won' t compile. If (X. Equals (y )) System. Out. println ("hi ");

Objects are more complex. The = operator checks whether two references point to the same object, while the equals method implements more specialized equality checks.

What is even more confusing is that the implementation of the default equals method provided by Java. Lang. Object uses = to easily determine whether the two objects to be compared are the same.

Many classes overwrite the default equals method to make it more useful, such as the string class. Its equals method checks whether two string objects contain the same string, the equals method of integer checks whether the int value is equal.

Most of the time, you should use the equals method to check whether two objects are equal. For atomic data, use the = Operator.

　　8. Common Errors 8 #: obfuscation of atomic and non-atomic operations

Java ensures that 32-bit read and write operations or smaller values are atomic operations, that is, they can be completed in one step and thus cannot be interrupted. Therefore, such read and write operations do not need to be synchronized. The following code is thread safe:

Public class example { Private int value; // more code here... Public void set (int x ){ // Note: No synchronized keyword This. value = X; } }

However, this guarantee is limited to read and write. The following code is not thread-safe:

Public void increment (){ // This is required tively two or three instructions: // 1) read current setting of 'value '. // 2) Increment that setting. // 3) write the new setting back. ++ This. value; }

During the test, you may not capture this error. First, it is difficult and time-consuming to test thread-related errors. Second, on some machines, the Code may be translated into a command, so it works normally. This error may only appear when it is tested on other virtual machines. Therefore, it is best to correctly synchronize the code at the beginning:

Public synchronized void increment (){ ++ This. value; }

　　9. Common Errors 9 # Clear a Catch Block

The following code clears a Catch Block:

Outputstream OS = NULL; Try { OS = new outputstream (); // Do something with OS here. OS. Close (); } Catch (exception e ){ If (OS! = NULL) OS. Close (); }

Although this code is problematic in several aspects, it is easy to miss this error during testing. The following lists three problems with this Code:

1. The statement OS. Close () appears in two places. In this case, maintenance is troublesome.

2. The above Code only handles exception, but does not involve error. However, when an error occurs during the try block operation, the stream should also be closed.

3. Close () may throw an exception.

A better version of the code above is:

Outputstream OS = NULL; Try { OS = new outputstream (); // Do something with OS here. } Finally { If (OS! = NULL) OS. Close (); }

This version eliminates the two problems mentioned above: the code is no longer duplicated, and the error can be correctly handled. But there is no good way to deal with the third problem. Maybe the best way is to put the close () statement in a try/Catch Block separately.

　　10. Common Errors 10 #: add unnecessary catch Blocks

After hearing the name of the try/Catch Block, some developers may assume that all try/catch blocks must have catch blocks that match them.

C ++ programmers especially think this way, because the concept of finally blocks does not exist in C ++, and the only reason for the existence of try blocks is to be paired with catch blocks.

The code for adding unnecessary catch blocks is like the following. The caught exception is immediately thrown:

Try { // Nifty code here } Catch (exception e ){ Throw E; } Finally { // Cleanup code here }

After unnecessary catch blocks are deleted, the above Code is shortened:

Try { // Nifty code here } Finally { // Cleanup code here }

Common Errors 11 #; equals, hashcode, or clone methods are not correctly implemented

The default implementation of methods equals, hashcode, and clone provided by Java. Lang. object is correct. Unfortunately, these default implementations are useless most of the time, so many classes override several of them to provide more useful functions. However, the problem arises again. When inheriting a parent class that overwrites several of these methods, subclasses usually need to overwrite these methods. During code review, make sure that if the parent class implements equals, hashcode, or clone methods, the subclass must also be correct. Correct implementation of equals, hashcode, and Clone requires some skills.

　　Summary

I have encountered these errors at least once during code review, and I have made several of them myself. The good news is that as long as you know what errors you are looking for, code reviews are easy to manage and errors are easily discovered and modified. Even if you cannot find time for formal code review, removing these errors from your code in self-audit mode will greatly save your debugging time. It is worthwhile to spend time reviewing code.