FindBugs helps learn Java better

Static code analyzers love for the fact that they help to find errors made by inattention. But it is much more interesting that they help to correct mistakes made out of ignorance. Even if everything is written in the official documentation for the language, it is not a fact that all programmers have carefully read this. And programmers can understand: all the documentation to read tormented.



In this regard, the static analyzer is similar to an experienced comrade who sits nearby and looks at how you write code. He not only tells you: “here you were mistaken when copy-paste”, but also says: “no, you can't write like that, look at the documentation yourself”. Such a friend is more useful than the documentation itself, because it tells only those things that you really encounter in your work, and is silent about those that you will never find useful.



In this post I will discuss some of the intricacies of Java, which I learned as a result of using the FindBugs static analyzer. Perhaps some things will be unexpected for you. It is important that all examples are not speculative, but are based on real code.

')

Ternary operator?:

It would seem that there is nothing easier than a ternary operator, but it has its own pitfalls. I thought that there was no fundamental difference between the constructions.

Type var = condition ? valTrue : valFalse; 

and

 Type var; if(condition) var = valTrue; else var = valFalse; 

It turned out that there is a subtlety. Since a ternary operator can be part of a complex expression, its result must be a specific type defined at compile time. Therefore, for example, with the true condition in the if form, the compiler leads valTrue to Type immediately, and in the form of a ternary operator it first leads to a common type valTrue and valFalse (although valFalse is not calculated), and then the result leads to Type. Coercion rules are not entirely trivial if primitive types and wrappers over them (Integer, Double, etc.) participate in the expression. All rules are described in detail in JLS 15.25 . Let's look at some examples.

 Number n = flag ? new Integer(1) : new Double(2.0); 

What will happen in n if the flag is set? Double object with a value of 1.0. Our clumsy attempts to create an object are ridiculous to the compiler. Since the second and third arguments are wrappers over different primitive types, the compiler expands them and results in a more accurate type (in this case, double). And after performing the ternary assignment operator, boxing is performed again. In essence, the code is equivalent to this:

 Number n; if( flag ) n = Double.valueOf((double) ( new Integer(1).intValue() )); else n = Double.valueOf(new Double(2.0).doubleValue()); 

From the point of view of the compiler, the code does not contain problems and compiles perfectly. But FindBugs gives a warning:

BX_UNBOXED_AND_COERCED_FOR_TERNARY_OPERATOR: Primitive value is unboxed and coerced for ternary operator in TestTernary.main (String [])



The conditional ternary operator has been wrapped up in a primitive value. The Semantics of Java and the E2 are wrapped up with the numeric values ​​and the E2 is the type of float, then the e1 is unboxed, converted to a floating point value, and boxed. See JLS Section 15.25.

Of course, FindBugs warns that Integer.valueOf (1) is more efficient than new Integer (1), but that's all they already know.



Or an example:

 Integer n = flag ? 1 : null; 

The author wants to put null in n if the flag is not set. Think it will work? Yes. But let's complicate:

 Integer n = flag1 ? 1 : flag2 ? 2 : null; 

It would seem that there is not much difference. Now, however, if both flags are cleared, this line generates a NullPointerException. The choices for the right ternary operator are int and null, therefore the resulting type is Integer. The options for the left are int and Integer, therefore, according to the rules of Java, the result is an int. To do this, you must perform unboxing by calling intValue, which throws an exception. The code is equivalent to this:

 Integer n; if( flag1 ) n = Integer.valueOf(1); else { if( flag2 ) n = Integer.valueOf(Integer.valueOf(2).intValue()); else n = Integer.valueOf(((Integer)null).intValue()); } 

Here, FindBugs gives two messages, which are enough to suspect an error:

BX_UNBOXING_IMMEDIATELY_REBOXED: Boxed value is unboxed and then immediately reboxed in TestTernary.main (String [])

NP_NULL_ON_SOME_PATH: Possible null pointer dereference of TestTernary.main (String [])

If there is a branch of the statement, it will be dereferenced, which will generate a nullPointerException.

Well, the last example on this topic:

 double[] vals = new double[] {1.0, 2.0, 3.0}; double getVal(int idx) { return (idx < 0 || idx >= vals.length) ? null : vals[idx]; } 

Not surprisingly, this code does not work: how can a function that returns a primitive type return null? It's amazing that it compiles without problems. Why compile - you already understand.

Dateformat

For formatting dates and times in Java, it is recommended to use classes that implement the DateFormat interface. For example, it looks like this:

 public String getDate() { return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(new Date()); } 

Often a class reuses the same format. Many will come up with the idea of ​​optimization: why create a format object every time when you can use a common instance?

 private static final DateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); public String getDate() { return format.format(new Date()); } 

That is so beautiful and cool, but unfortunately it does not work. It works more precisely, but occasionally it breaks. The fact is that the documentation for DateFormat says :

Date formats are not synchronized. It is recommended to create a separate format instances for each thread. If multiple threads access a concurrently, it must be synchronized externally.

And this is true, if you look at the internal implementation of SimpleDateFormat. During the execution of the format () method, the object writes to the class fields; therefore, using SimpleDateFormat from two threads simultaneously will result in an incorrect result with some probability. Here is what FindBugs writes about this:

STCAL_INVOKE_ON_STATIC_DATE_FORMAT_INSTANCE: Call to method of static java.text.DateFormat in TestDate.getDate ()

As the JavaDoc states, DateFormats are inherently unsafe for multithreaded use. The detector has found that it has been obtained through a static field. This looks suspicous.



Sun Bug # 6231579 and Sun Bug # 6178997 .

Pitfalls BigDecimal

Having learned that the BigDecimal class allows storing fractional numbers of arbitrary precision, and seeing that it has a double constructor, some will decide that everything is clear and you can do this:

 System.out.println(new BigDecimal(1.1)); 

Nobody really forbids doing this, only the result may seem unexpected: 1.1000000000000000888178419700125232338903333727265625. This happens because primitive double is stored in the IEEE754 format, in which it is impossible to imagine 1.1 perfectly accurate (in binary notation, an infinite periodic fraction is obtained). Therefore, the closest value to 1.1 is stored there. On the other hand, the BigDecimal (double) constructor works exactly: it ideally converts a given number in IEEE754 to decimal form (the final binary fraction is always representable as the final decimal). If you want to represent just 1.1 in the form of BigDecimal, then you can write either new BigDecimal("1.1") or BigDecimal.valueOf(1.1) . If the number is not output immediately, and you can do some operations with it, you may not understand where the error comes from. FindBugs issues a DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE warning, which gives the same advice.



And here's another thing:

 BigDecimal d1 = new BigDecimal("1.1"); BigDecimal d2 = new BigDecimal("1.10"); System.out.println(d1.equals(d2)); 

In fact, d1 and d2 are the same number, but equals returns false, because it compares not only the value of numbers, but also the current order (the number of decimal places). This is written in the documentation, but few will read the documentation for such a familiar method as equals. Such a problem can emerge far from immediately. FindBugs itself, unfortunately, does not warn about this, but there is a popular extension to it - fb-contrib , in which this bug is taken into account:

MDM_BIGDECIMAL_EQUALS



equals () being two java.math.BigDecimal numbers. It is normally a rule. To compare BigDecimal objects for mathematical equality, use CompareTo () instead.

Strings and printfs

Often programmers who switch to Java after C will happily discover PrintStream.printf (and also PrintWriter.printf , etc.). Like, well, I know that, just like in C, nothing new needs to be taught. In fact, there are differences. One of them is in the translation of strings.



In the C language, there is a division into text and binary streams. The output of the character '\ n' to a text stream in any way will automatically be converted to a system-dependent newline ("\ r \ n" on Windows). In Java, there is no such separation: you must pass the correct sequence of characters to the output stream. This is automatically done, for example, by the methods of the PrintStream.println family. But when using printf, passing '\ n' in the format string is just '\ n', not a system-dependent newline. For example, write the following code:

 System.out.printf("%s\n", "str#1"); System.out.println("str#2"); 

Redirecting the result to the file, we will see:



Thus, you can get a strange combination of line breaks in one thread, which looks messy and can tear down the roof to some parser. You can ignore the error for a long time, especially if you mostly work on Unix-systems. To insert the correct line feed using printf, the special format character "% n" is used. Here is what FindBugs writes about this:

VA_FORMAT_STRING_USES_NEWLINE: Format string should not use% n rather than \ n in TestNewline.main (String [])



This format string includes a newline character (\ n). In the format of the strings, it will be more convenient to use.

Perhaps for some readers all of the above was known for a long time. But I am pretty sure that there will be an interesting warning from them for the static analyzer, which will reveal to them the new features of the programming language used.