...
This noncompliant code example is designed to rename a given file if it the file to be renamed is present, perform operations on itthe renamed file, and then delete itthe renamed file. However, due to the short-circuit behavior of the ||
operator, the renameTo()
method does not execute appearing as the second argument to ||
is not executed when the exists()
method appearing as the first argument to ||
returns true
, and an unsuspecting developer would incorrectly attempt to delete the non-existing fNew
file instead of the original file fOriginal
. Because of this, the renamed file may or may not exist, which may result in an attempt to use and then delete a nonexistent file. This problem is exacerbated by the fact that File.delete()
does not throw an exception but returns an error code on failure, which is often silently ignored or perceived as unnecessary. (See EXP02-J. Do not ignore values returned by methods)
Code Block | ||
---|---|---|
| ||
class BadRenameFile { public static void main(String[] args) { File fOriginal = new File("original.txt"); File fNew = new File("new.txt"); if(fOriginal.exists() || fOriginal.renameTo(fNew)) { // do something with fNew, which is not guaranteed to exist. fNew will only // exist if file 'new.txt' exists, which has not been checked by this program. fNew.delete();// ... // fNew doesmay not exist as renameTo() ismay not have been successfully executed and // the existence of 'new.txt' has not been checked. fNew.delete(); } } } |
Compliant Solution
Knowledge of the short-circuit behavior can be used to enforce the desired specification. This program traps an error if the file does not exist or when it cannot be renamed to the new file name. Operations on the new file follow.
...
This noncompliant example differs from the previous one in that, there are no side effects in the right hand side operand. Nevertheless, an unaware programmer can get caught in the short-circuit behavior of the conditional AND and OR operators. The programmer has combined two expressions in the if
statement. The first checks whether the d
object is null
and the second checks if the default security manager has been installed (by comparing sm
with null
) depending on which . If both d
and sm
are not null
, the security check will be performed. This is a case of trying to combine together two null
checks into one statement. A conditional &&
is used as using a conditional ||
would mean that whenever d
is null
, the complete expression can still succeed depending on the value of sm
. Using the ||
operator violates the invariants of d
as it is desired that operations on it be prohibited if it is null
.
The result of this expression is that if On the other hand, when &&
is used and d
is equal to null
, the current if
expression evaluates to false
and the security check is not executed.
...
Code Block |
---|
if (data != null && i < data.length && data[i] != -1) ... |
This code snippet sequentially executes the subexpressions while avoiding an array indexing exception resulting from the checks that execute prior to the last subexpression.
...