低速ディテクタです。
]]>java.net.URL
のequals()とhashCode()はドメイン名の解決を必要とします。結果として、これらは非常に重い処理となるので、このディテクタは、これらのメソッドの呼び出しを発見します。
]]>
|
と &
)を、短絡的条件演算子(||
と &&
)と間違えて使用していると思われるコードを発見します。
]]>
Comparator
を実装するクラスが守るべきイディオムを守っていないクラスを発見します。高速ディテクタです。
]]>
java.util.concurrent
)
ロックが、このメソッドを起点とする全てのパスで解放されているかどうか調べます。
中速ディテクタです。このディテクタを利用するには、
java.util.concurrent
パッケージを外部クラスパスに指定する必要があります(このパッケージ自身を調査する場合を除く)。
]]>
java.lang.String
のように)ケースを発見します。低速ディテクタです。
]]>
高速ディテクタです。
]]>高速ディテクタです。
]]>高速ディテクタです。
]]>低速ディテクタです。
]]>高速ディテクタです。
]]>高速ディテクタです。
]]>高速ディテクタです。
]]>低速ディテクタです。
]]>低速ディテクタです。
]]>低速ディテクタです。
]]>java.lang.Object
を受け取る汎用型メソッドの引数を調査し、これがコンテナの型パラメータと関連しているかチェックします。関係の無い型のオブジェクトをコンテナに格納することはできません。例えばfoo
がList<String>
であり、bar
がStringBuffer
の場合、foo.contains(bar)
は常にfalseを返します。高速ディテクタです。
]]>
FindBugs looks only for the most blatent, obvious cases of HTTP response splitting. If FindBugs found any, you almostly certainly have more vulnerabilities that FindBugs doesn't report. If you are concerned about HTTP response splitting, you should seriously consider using a commercial static analysis or pen-testing tool, such as those provided by Fortify Software, a sponsor of the FindBugs project. If your software is open source, Fortify will scan your code for free as part of the JOR (Java Open Review) effort.
]]>FindBugs looks only for the most blatent, obvious cases of HTTP response splitting. If FindBugs found any, you almostly certainly have more vulnerabilities that FindBugs doesn't report. If you are concerned about HTTP response splitting, you should seriously consider using a commercial static analysis or pen-testing tool, such as those provided by Fortify Software, a sponsor of the FindBugs project. If your software is open source, Fortify will scan your code for free as part of the JOR (Java Open Review) effort.
]]>FindBugs looks only for the most blatent, obvious cases of cross site scripting. If FindBugs found any, you almostly certainly have more cross site scripting vulnerabilities that FindBugs doesn't report. If you are concerned about cross site scripting, you should seriously consider using a commercial static analysis or pen-testing tool, such as those provided by Fortify Software, a sponsor of the FindBugs project. If your software is open source, Fortify will scan your code for free as part of the JOR (Java Open Review) effort.
]]>FindBugs looks only for the most blatent, obvious cases of cross site scripting. If FindBugs found any, you almostly certainly have more cross site scripting vulnerabilities that FindBugs doesn't report. If you are concerned about cross site scripting, you should seriously consider using a commercial static analysis or pen-testing tool, such as those provided by Fortify Software, a sponsor of the FindBugs project. If your software is open source, Fortify will scan your code for free as part of the JOR (Java Open Review) effort.
]]>FindBugs looks only for the most blatent, obvious cases of cross site scripting. If FindBugs found any, you almostly certainly have more cross site scripting vulnerabilities that FindBugs doesn't report. If you are concerned about cross site scripting, you should seriously consider using a commercial static analysis or pen-testing tool, such as those provided by Fortify Software, a sponsor of the FindBugs project. If your software is open source, Fortify will scan your code for free as part of the JOR (Java Open Review) effort.
]]>this.getClass().getResource(...)
の呼び出しは、別のパッケージに継承クラスがある場合には、予期しない結果をもたらす可能性があります。
]]>
putNextEntry()
をcloseEntry()
呼び出しのすぐあとに呼び出しています。これは、空のzipファイルエントリを生成してしまいます。zipファイルへのエントリ書き込みはputNextEntry()
とcloseEntry()
の間に行う必要があります。
]]>
putNextEntry()
をcloseEntry()
呼び出しのすぐあとに呼び出しています。これは、空のjarファイルエントリを生成してしまいます。jarファイルへのエントリ書き込みはputNextEntry()
とcloseEntry()
の間に行う必要があります。
]]>
全てのclone()メソッドがsuper.clone()を呼び出すようになっていれば、Object.clone()が呼び出される事が保証され、正しい型のオブジェクトが返送されます。
]]>java.net.URI
を使用することを検討してください。
]]>
java.net.URI
を使用することを検討してください。
]]>
java.lang.String(String)
コンストラクタの呼び出しはメモリを浪費するだけです。このようにして生成されたオブジェクトと元のString
オブジェクトは機能的に区別が付きません。
]]>
java.lang.String
の引数無しコンストラクタの呼び出しはメモリを浪費するだけです。このようにして生成されたオブジェクトと、空文字列""
の間には機能的に違いがありません。Javaは、同一内容の文字列定数のインスタンスを1つにまとめます。従って単に空文字列を直接使用すべきです。
]]>
String.toString()
を明示的に呼び出すのは冗長です。
元のStringをそのまま使用してください。
]]>過去において、close()やfinalize()メソッドでのガベージコレクションを明示的に呼び出すことが、パフォーマンスのブラックホールに陥れるケースがありました。ガベージコレクションは高くつく場合があります。数百、数千のガベージコレクション呼び出しは、システムのパフォーマンスを極めて落とす事になります。
]]>java.lang.Boolean
のインスタンスを新規に生成するのはメモリの浪費です。Boolean
クラスはイミュータブルなので、TRUEとFALSEの2つのオブジェクトがあれば十分です。 Boolean.valueOf()
(あるいは、Java 5以上ならばオートボクシング)を替わりに使ってください。
]]>
new Integer(int)
の呼び出しは、常に新たなオブジェクトが生成されます。これに対してInteger.valueOf(int)
は、コンパイラ、クラスライブラリ、JVMによってキャッシュすることが可能です。キャッシュを使用することによって、余計なオブジェクトの生成を防ぎ、コードの実行効率を改善できます。
-128から127までの値は、キャッシュされるので、valueOf
を使用すると約3.5倍高速になります。この範囲外の場合にはパフォーマンスの差はありません。
バージョン5.0より前のJava実行環境との互換性が不要であれば、オートボクシングか、Long
、Integer
、Short
、Character
、Byte
のvalueOf()
メソッドを代わりに使用してください。
new Double(double)
を呼び出すと、常に新たなオブジェクトが生成されます。これに対し、Double.valueOf(double)
は、コンパイラ、クラスライブラリ、JVMによってキャッシュすることが可能です。キャッシュを使用することによって、余計なオブジェクトの生成を防ぎ、コードの実行効率を改善できます。
バージョン5.0より前のJava実行環境との互換性が不要であれば、オートボクシングか、Double
、Float
のvalueOf()
メソッドを代わりに使用してください。
String.toUpperCase(Locale l) |
String.toLowerCase(Locale l) |
を使用してください。
]]> b ? e1 : e2
operator). The
semantics of Java mandate that if e1
and e2
are wrapped
numeric values, the values are unboxed and converted/coerced to their common type (e.g,
if e1
is of type Integer
and e2
is of type Float
, then e1
is unboxed,
converted to a floating point value, and boxed. See JLS Section 15.25.
]]>
new Double(d).intValue()
)。プリミティブ型のみを用いて、直接型変換してください(例:(int) d
)。
]]>
置換前 | 置換後 |
---|---|
new Integer(1).toString() | Integer.toString(1) |
new Long(1).toString() | Long.toString(1) |
new Float(1.0).toString() | Float.toString(1.0) |
new Double(1.0).toString() | Double.toString(1.0) |
new Byte(1).toString() | Byte.toString(1) |
new Short(1).toString() | Short.toString(1) |
new Boolean(true).toString() | Boolean.toString(true) |
java.util.concurrent.locks.Condition
objectのwait()
を呼び出しています。Condition
オブジェクトで待ち合わせる場合はCondition
インターフェースに用意されたawait()
のどれかを使用すべきです。
]]>
finalize()
メソッドはpublicになっていますが、protectedであるべきです。
]]>
finalize()
メソッドは無駄です。削除するべきです。
]]>
finalize()
メソッドが空なので、親クラスのファイナライザを無視する事になり、親クラスで定義されたファイナライザの処理が呼び出されなくなってしまいます。そのような意図が無ければ、このメソッドを削除してください。
]]>
finalize()
メソッドは、親クラスのfinalize()
を呼び出しているだけで、無駄です。削除してください。
]]>
finalize()
メソッドは、親クラスのfinalize()
を呼び出していません。このため親クラスで定義されたファイナライズ処理が行われません。super.finalize()
を呼び出すように変更してください。
]]>
finalize()
の明示的な呼び出しが行われています。ファイナライザは一度だけ呼び出されるべきで、それはVMによって行われるので、このコードの意図は間違っています。
もしも参照によってつながった複数のオブジェクトがファイナライズされると、それぞれのオブジェクトのfinalize()がVMから呼び出されますが、もしかすると別々のスレッドによって複数のfinalize()が同時に呼び出されるかもしれません。このため、あるクラスXのfinalize()の中から、別のオブジェクト(クラスXから参照されている)のfinalize()を呼び出すのは、とりわけ間違ったやり方です。なぜならすでにそのオブジェクトは別のスレッドでファイナライズ済みかもしれないからです。
]]>equals()
メソッドが定義されていますが、Objectクラスのequals(Object)
が使用されています。
このクラスは、おそらく共変でないequals()
を定義すべきです。
(すなわち、メソッドシグニチャを次のようにすべきです boolean equals(java.lang.Object)
。
]]>
equals()
method, that doesn't override the normal equals(Object)
method
defined in the base java.lang.Object
class.
The class should probably define a boolean equals(Object)
method.
]]>
equals()
method, that doesn't override the normal equals(Object)
method
defined in the base java.lang.Object
class. Instead, it
inherits an equals(Object)
method from a superclass.
The class should probably define a boolean equals(Object)
method.
]]>
equals()
メソッドが定義されています。
java.lang.Object
のequals()
メソッドを正しくオーバーライドするには、equals()
メソッドの引数は、java.lang.Object
でなければなりません。
]]>
instanceof
in the determination of whether two objects are equal. This is fraught with peril,
since it is important that the equals method is symetrical (in other words, a.equals(b) == b.equals(a)
).
If B is a subtype of A, and A's equals method checks that the argument is an instanceof A, and B's equals method
checks that the argument is an instanceof B, it is quite likely that the equivalence relation defined by these
methods is not symmetric.
]]>
Foo
it might check if Foo.class == o.getClass()
).
It is better to check if this.getClass() == o.getClass()
.
]]>
this
object. There might not be anything wrong with
this code, but it is worth reviewing.
]]>
The likely intended semantics are object identity: that an object is equal to itself. This is the behavior inherited from class Object
. If you need to override an equals inherited from a different
superclass, you can use use:
public boolean equals(Object o) { return this == o; }]]>
java.lang.Object
でないcompareTo()
メソッドを定義しています。
Comparable
インターフェースのcompareTo()
メソッドを正しく実装するには、compareTo()
メソッドの引数の型は、java.lang.Object
でなければなりません。
]]>
hashCode()
メソッドを定義していますが、equals()
メソッドはjava.lang.Object
のもの(参照が同一かどうかで同値性を判定します)をそのまま利用しています。これは、同値であると判定されたオブジェクトが同じハッシュコードを返さなければならないという規約は満たしますが、恐らくhashCode()
メソッドのオーバーライドは意図されたものでは無いと思われます(hashCode()
のオーバーライドは、単なる参照が同一という同値性よりも複雑な基準がある事を意味します)。
もしもこのクラスのインスタンスがHashMap/HashTableに挿入されることがあり得ないと考えるのであれば、推奨されるhashCodeの実装は次のようになります。
public int hashCode() { assert false : "hashCodeが呼び出されることは想定されていません。"; return 42; // 適当な値 }]]>
compareTo(...)
を宣言していますがequals()
をjava.lang.Object
から継承しています。一般にcompareToが0を返す条件は、equalsがtrueを返す条件と一致する必要があります。これを守らないと複雑で予測不可能な問題が、例えばPriorityQueueで発生するでしょう。Java 5では、PriorityQueue.remove()は、compareTo()を使用していますがJava 6では、equals()を使用しています。
ComparableインターフェースのcompareToメソッドのJavaDocを以下に引用します。
必須というわけではありませんが、(x.compareTo(y)==0) == (x.equals(y))
であることが強く推奨されます。一般に、Comparable インタフェースを実装しているクラスで、この条件に違反するクラスはすべて、明確にこの事実を示す必要があります。「注:このクラスは equals と一貫性のない自然順序付けを持ちます」などと明示することをお勧めします。
]]>
hashCode()
メソッドを定義していますが、equals()
メソッドを定義していません。このため、同値のオブジェクトは同じハッシュコードを返すべきという規約を破る可能性があります。
]]>
equals(Object)
をオーバーライドしていますが、hashCode()
をオーバーライドしていないので、java.lang.Object
のhashCode()
の実装をそのまま利用しています(これは、VMがObjectに割り当てた一意のハッシュコードを返します)。このため、同値のオブジェクトが等しいハッシュコードを返さなければならないという規約を破る可能性が非常に高いです。
もしもこのクラスのインスタンスがHashMap/HashTableに挿入されることがあり得ないと考えるのであれば、推奨されるhashCodeの実装は次のようになります。
public int hashCode() { assert false : "hashCodeが呼び出されることは想定されていません。"; return 42; // 適当な値 }]]>
equals(Object)
を親の抽象クラスから継承しhashCode()
は、java.lang.Object
クラスのものをそのまま使用しています(これは、VMによってアサイン
された任意の値を返送します)。このクラスは「等しいオブジェクトが、同値のハッシュコードを返さなければならない」
という契約を満たすのが非常に困難になっています。
もしもhashCodelを定義する必要がない、あるいはこのオブジェクトをHashMap/Hashtableに格納したく
ないと考えるのであれは、hashCode()
メソッドをUnsupportedOperationException
を
スローするように実装してください。
equals(Object)
をオーバーライドしていますが、hashCode()
をオーバーライドしていません。このため、このクラスは同値のオブジェクトが等しいハッシュコードを返さなければならないという規約を破る可能性があります。
]]>
equals()
メソッドが定義されています。
java.lang.Object
のequals()
メソッドを正しくオーバーライドするには、equals()
メソッドの引数は、java.lang.Object
でなければなりません。
]]>
java.lang.String
のオブジェクトは==もしくは!=を使って参照の比較を行っています。定数定義された文字列もしくは、String.intern()
で得られた文字列でなければ、同じ内容でも別のオブジェクトとなる可能性があります。equals(Object)
メソッドを替わりに用いる事を検討してください。
]]>
java.lang.String
のパラメータを==、!=を使用して比較しています。これは呼び出し元が文字列定数か、internされた文字列しか渡せないことを意味しており、非常に脆いものになっています。このようにしても実行効率上のメリットはほとんどありません。equals(Object)
を代わりに使用することを検討してください。
]]>
java.lang.Object
でないcompareTo()
メソッドを定義しています。
Comparable
インターフェースのcompareTo()
メソッドを正しく実装するには、compareTo()
メソッドの引数の型は、java.lang.Object
でなければなりません。
]]>
このパターンに当てはまる典型的な例は、スレッドセーフを意図しているのに、メソッドを同期化するのを忘れているケースです。
ディテクタにコードの意図を伝えるため「非同期アクセス」のラベルを付ける事ができます。
このディテクタには、判断を誤る要因が幾つかあります。例えば、ディテクタはスレッドがロックを獲得しているかどうかを、コードを見て静的に判断する事は出来ません。例えディテクタが、ロックされたアクセスと、そうでないアクセスを的確に判定出来たとしても、依然としてコードが正しい可能性は残ります。
ここの記述は"IS2"バージョンのパターンディテクタを参考にしています。これは、ロック・非ロックアクセスの検出に関して"IS"バージョンよりも正確です。
]]>notify()
もしくはnotifyAll()
メソッドを呼び出しています。一般にはこれらのメソッドは、別のスレッドが待ち合わせている条件が成立したのを知らせるために用いられます。条件の待ち合わせを行うためには、両方のスレッドから見る事が可能なヒープオブジェクトを使用しなければなりません。
このバグ報告は、必ずしもプログラミングエラーを意味しません。オブジェクトの状態変更がメソッド内部で行われ、そこから更にnotify()/notifyall()メソッドを持ったメソッドを呼び出しているかもしれないからです。
]]>run()
メソッドを明示的に呼び出しています。一般にRunnable
を実装したクラスは、新しいスレッドがrun()
メソッドを呼び出す事を期待しており、この場合、Thread.start()
を呼び出すのが正しいやり方です。
]]>
非短絡論理は、両側の式が評価されます。それはたとえ左側だけで結果が決定してしまう場合にもです。これは効率が悪く、左側がガード条件になっている場合には、右側の評価でエラーが発生するかもしれません。
詳細はJava言語仕様を参照してください。
]]>詳細については、Java言語仕様を参照してください。
]]>java.lang.Object.wait()
を呼び出していますが、条件判断によってガードされていません。wait()を呼び出す前に、待ち条件が既に成立していないかどうか、調査する必要があります。wait()呼び出し前に行われた通知は無視されてしまいます。
]]>
foo
は、まだnullの状態になるでしょう。
public class CircularClassInitialization { static class InnerClassSingleton extends CircularClassInitialization { static InnerClassSingleton singleton = new InnerClassSingleton(); } static CircularClassInitialization foo = InnerClassSingleton.singleton; }]]>
java.util.Iterator
を実装していますが、next()
メソッドがjava.util.NoSuchElementException
をスロー出来ないようになっています。next()
メソッドは、もう返送出来るエレメントが無くなった際にNoSuchElementException
をスローしなければなりません。
]]>
private static String LOCK = "LOCK"; ... synchronized(LOCK) { ...} ...
Constant Strings are interned and shared across all other classes loaded by the JVM. Thus, this could is locking on something that other code might also be locking. This could result in very strange and hard to diagnose blocking and deadlock behavior. See http://www.javalobby.org/java/forums/t96352.html and http://jira.codehaus.org/browse/JETTY-352.
]]>private static Boolean inited = Boolean.FALSE; ... synchronized(inited) { if (!inited) { init(); inited = Boolean.TRUE; } } ...
Since there normally exist only two Boolean objects, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock
]]>private static final Integer fileLock = new Integer(1); ... synchronized(fileLock) { .. do something .. } ...
It would be much better, in this code, to redeclare fileLock as
private static final Object fileLock = new Object();The existing code might be OK, but it is confusing and a future refactoing, such as the "Remove Boxing" refactoring in IntelliJ, might replace this with the use of an intern'd Integer object shared throughout the JVM, leading to very confusing behavior and potential deadlock. ]]>
private static Integer count = 0; ... synchronized(count) { count++; } ...
Since Integer objects can be cached and shared, this code could be synchronizing on the same object as other, unrelated code, leading to unresponsiveness and possible deadlock
]]>synchronized() {}
空のsynchronizedブロックは難解で、考えられている程利用は容易ではありません。また少々ぶかっこうなコードになっても大抵の場合、空のsynchronizedブロックを使用しない方が、より優れたコードになります。
]]>このパターンに当てはまる典型的な例は、スレッドセーフを意図しているのに、メソッドを同期化するのを忘れているケースです。
このディテクタには、判断を誤る要因が幾つかあります。例えば、ディテクタはスレッドがロックを獲得しているかどうかを、コードを見て静的に判断する事は出来ません。例えディテクタが、ロックされたアクセスと、そうでないアクセスを的確に判定出来たとしても、依然としてコードが正しい可能性は残ります。
]]>private Long myNtfSeqNbrCounter = new Long(0); private Long getNotificationSequenceNumber() { Long result = null; synchronized(myNtfSeqNbrCounter) { result = new Long(myNtfSeqNbrCounter.longValue() + 1); myNtfSeqNbrCounter = new Long(result.longValue()); } return result; }]]>
alpha.Foo
extends beta.Foo
).
This can be exceptionally confusing, create lots of situations in which you have to look at import statements
to resolve references and creates many
opportunities to accidently define methods that do not override methods in their superclasses.
]]>
alpha.Foo
extends beta.Foo
).
This can be exceptionally confusing, create lots of situations in which you have to look at import statements
to resolve references and creates many
opportunities to accidently define methods that do not override methods in their superclasses.
]]>
import alpha.Foo; public class A { public int f(Foo x) { return 17; } } ---- import beta.Foo; public class B extends A { public int f(Foo x) { return 42; } }
The f(Foo)
method defined in class B
doesn't
override the
f(Foo)
method defined in class A
, because the argument
types are Foo
's from different packages.
import alpha.Foo; public class A { public int f(Foo x) { return 17; } } ---- import beta.Foo; public class B extends A { public int f(Foo x) { return 42; } public int f(alpha.Foo x) { return 27; } }
The f(Foo)
method defined in class B
doesn't
override the
f(Foo)
method defined in class A
, because the argument
types are Foo
's from different packages.
In this case, the subclass does define a method with a signature identical to the method in the superclass, so this is presumably understood. However, such methods are exceptionally confusing. You should strongly consider removing or deprecating the method with the similar but not identical signature.
]]>hashcode()
というメソッドが定義されています。このメソッドは、java.lang.Object
のhashCode()
を(そのように意図したのかもしれませんが)オーバーライドしません。
]]>
tostring()
を定義しています。このメソッドは、java.lang.Object
のtoString()
を(そのように意図したのかもしれませんが)オーバーライドしません。
]]>
equal(Object)
を定義しています。このメソッドは、java.lang.Object
のequals(Object)
を(そのように意図したのかもしれませんが)オーバーライドしません。
]]>
java.io.InputStream.read()
の戻り値を無視しています。戻り値を無視すると、実際に何バイトのデータが読み込まれたのか分かりません。一般には、指定された長さ分完全に読み込んでしまうケースが多いために、顕在化せず、ごくたまに発生するやっかいなバグとなります。
]]>
java.io.InputStream.skip()
の戻り値を無視しています。
戻り値をチェックしないと、実際には呼び出し元が要求したバイト数よりも少ないバイト数しかスキップしなかった場合に、何バイトスキップしたのか分らなくなります。
これは潜在的なバグとなります。なぜなら、大抵の場合、要求通りのスキップが行なわれるので、まれにしか現象が起きないからです。
これに対し、バッファストリームの場合、skip()はバッファ内部のデータをスキップするだけなので、頻繁に現象が起きることになります。
]]>
Serializable
インターフェースを実装しており、直列化、直列化復元を行うためのメソッドを宣言しています。しかしメソッドがprivate宣言されていないので単に無視されます。
]]>
Externalizable
インターフェースを実装していますが、引数無しのコンストラクタを定義していません。Externalizableオブジェクト直列化復元される際、まず引数無しコンストラクタを呼んでインスタンスを生成する必要があります。このクラスには、これが無いので、実行時に直列化、直列化復元処理に失敗します。
]]>
Serializable
インターフェースを実装していますが、親クラスは実装していません。このクラスのオブジェクトを直列化復元する場合、親クラスのフィールドの初期化は、親クラスの引数無しコンストラクタで行う必要があります。ところが、親クラスが引数無しコンストラクタを持たないので、直列化、直列化復元処理は実行時に失敗します。
]]>
Serializable
を実装しています。しかし、serialVersionUID
フィールドを定義していません。クラス参照を追加する程度の簡単な変更でも、合成フィールドを追加することになり、これは、暗黙的に生成されるserialVersionUIDの値を変えてしまいます(例えば、String.class
への参照を追加すると、class$java$lang$String
というスタティックフィールドが生成されます)。また、複数のJavaコンパイラの間では、このクラス参照、インナークラス参照に対して生成される、合成フィールドの命名規則が異なる場合があります。異なるバージョン間での相互運用性を保証するため、serialVersionUIDを明示的に定義する事を検討してください。
]]>
Comparator
インターフェースを実装しています。Serializable
も実装すべきかどうか検討すべきです。もしもTreeMap
のような順序付きコレクションでこのコンパレータを使用すると、コンパレータが直列化可能である場合にのみ、TreeMap
は直列化されます。大抵のコンパレータは全く状態を持っていないか、持っていたとしてもわずかなので、直列化可能とするのは簡単であり、良い防御的プログラミングであると言えます。
]]>
writeObject()
メソッドは同期化されていますが、他のメソッドは同期化されていません。
]]>
readObject()
を定義していますが、そもそもオブジェクトが直列化復元される場合、ただ1つのスレッドによって行われる事になっています。このため、readObject()
を同期化する必要はありません。もしも readObject()
メソッド自体が、他のオブジェクトへの他のスレッドからのアクセスを引き起こしているなら、それは非常に疑わしいコーディングです。
]]>
serialVersionUID
フィールドを定義していますが、staticではありません。直列化のバージョンUIDのために作成したのであれば、このフィールドは、staticとすべきです。
]]>
serialVersionUID
フィールドを定義しますが、finalではありません。直列化のバージョンUIDのために作成したのであれば、このフィールドは、finalとすべきです。
]]>
serialVersionUID
フィールドを定義していますがlongではありません。直列化のバージョンUIDのために作成したのであれば、このフィールドは、longとすべきです。
]]>
java.lang.Object
でもないインスタンスフィールドを持っています。また、Externalizable
インターフェースも実装していませんし、readObject()
も writeObject()
メソッドも定義していません。このフィールドに実際に非直列化可能クラスのインスタンスを保持している場合、このクラスのオブジェクトは、直列化復元を正しく行えません。
]]>
可能であれば、インナークラスをstaticとしてください。outerクラスを直列化可能とすることでも問題を解決できますが、この場合インナークラスの直列化にあたってouterクラスも直列化されることに注意してください。おそらくこれはプログラマの意図とは異なるでしょう。 ]]>
java.lang.Object.wait()
を呼び出していますが、ループの中にありません。幾つかの条件を待ち合わせるためにモニタを利用する場合は、wait()を抜けたからといって、自分が待っている条件が成立している保証はありません。
]]>
java.util.concurrent.locks.Condition.await()
(あるいはその変形版) を呼び出していますが、ループの中にありません。オブジェクトが複数の条件の待ち合せに使用されると、起こされた時に、それが自分の待ち合わせ条件に合致するとは限りません。
]]>
notifyAll()
ではなくnotify()
を呼び出しています。Javaのモニタは、しばしば幾つかの条件を同時に待ち合わせるために使用されますが、notify()
を使って待機スレッドを起こすと、別の条件を待っているスレッドを起こすだけになるかもしれません。
]]>
String dateString = getHeaderField(name); dateString.trim();
このメソッドの戻り値をチェックするべきです。このようなコードを書いてしまった原因の1つとして、イミュータブルなオブジェクトのメソッド呼び出しが、そのオブジェクトの状態を変化させると誤解しているケースが考えられます。例えば、
]]>String dateString = getHeaderField(name); dateString = dateString.trim();
File.delete()
method returns false
if the file could not be successfully deleted (rather than
throwing an Exception).
If you don't check the result, you won't notice if the method invocation
signals unexpected behavior by returning an atypical return value.
]]>
if (x < 0) new IllegalArgumentException("x must be nonnegative");
It was probably the intent of the programmer to throw the created exception:
]]>if (x < 0) throw new IllegalArgumentException("x must be nonnegative");
String dateString = getHeaderField(name); dateString.trim();
このプログラマは、trim()メソッドがdateStringが参照しているStringの状態を変えると考えているように思われます。しかしStringはイミュータブルであり、trim()メソッドは新しいStringオブジェクトを返すので、これでは戻されたオブジェクトが捨てられてしまいます。このコードは、以下のように修正すべきです。
]]>String dateString = getHeaderField(name); dateString = dateString.trim();
NullPointerException
が発生するでしょう。
]]>
NullPointerException
が発生するでしょう。現在のFindBugsは、実行され得ない例外処理経路を考慮しません。このため、この警告は誤って報告されるかもしれません。
また、FindBugsはswitch-caseのdefaultも例外経路と見なしますが、しばしばdefaultは実行され得ない場合がありますので、注意してください。
]]>NullPointerException
を発生させる可能性があります。
]]>
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.
Due to the fact that this value had been previously tested for nullness, this is a definite possiblity.
]]>
NullPointerException
を発生させる可能性があります。現在のFindBugsは、実行され得ない例外処理系を考慮しません。このため、この警告は誤って報告されるかもしれません。
また、FindBugsはswitch-caseのdefaultも例外経路と見なしますが、しばしばdefaultは実行され得ない場合がありますので、注意してください。
]]>NullPointerException
を招きます。この値はnullを返す可能性のあるメソッドの戻り値を使用しているため、nullとなるかもしれません。
]]>
finally
ブロックを使ってリソースをクローズするのが良いやり方です。
]]>
finally
ブロックを使用するのが良い考えです。
]]>
これに対し「返すべき値が無い事を示す」のにnullを用いるのが、恐らく適切です。例えば、File.listFiles()
は、ディレクトリにファイルが無い時はサイズ0の配列を、指定されたパスがディレクトリでなければnullを返します。
if
ブロックを記述してしまった事が原因です。
if (argv.length == 1) { // TODO: handle this case }]]>
if
文を作成してしまった場合に起き得ます。
if (argv.length == 1); System.out.println("Hello, " + argv[0]);]]>
この警告は、一般には、明らかにnullでない値をnullとチェックすることにより報告されます。単純に、実際には、チェックは全く不要な、防御的プログラミングかもしれません。
]]>java.util.concurrent
) ロックを獲得していますが、このメソッドを起点とした実行経路に解放されない経路があります。一般には、このロックを使うための、正しいイディオムは次のようになります。
Lock l = ...; l.lock(); try { // do something } finally { l.unlock(); }]]>
java.util.concurrent
) ロックを獲得していますが、このメソッドを起点とした例外処理経路に、解放されない経路があります。一般には、このロックを使うための、正しいイディオムは次のようになります。
Lock l = ...; l.lock(); try { // do something } finally { l.unlock(); }]]>
false
を返します。
]]>
IllegalMonitorStateException
が発生します。
]]>
IllegalMonitorStateException
が発生します。
]]>
public void foo() { int x = 3; x = x; }この例のような代入は、無駄です。ロジックの誤りや、タイプミスかもしれません。 ]]>
int x; public void foo() { x = x; }この例のような代入は、無駄です。ロジックの誤りや、タイプミスかもしれません。 ]]>
int x,y; public void foo() { x = x = 17; }
フィールドに2回代入するのは無意味です。恐らく論理の誤りかタイプミスでしょう。
]]>public void foo() { int x,y; x = x = 17; }
変数に2回代入するのは無意味です。恐らく論理の誤りかタイプミスでしょう。
]]>生成される乱数値が推測できないようにするためには、毎回Randomオブジェクトを生成してはいけません(毎回生成すると、容易に推測できてしまいます)。またjava.security.SecureRandomを替わりに使用することについても真剣に検討すべきです(この場合も毎回インスタンスを生成してはいけません)。
]]>Integer.MIN_VALUE
だと、絶対値は負になります(なぜならMath.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE
だからです)。
]]>
Integer.MIN_VALUE
だと、絶対値は負になります(なぜならMath.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE
だからです)。
]]>
計算結果が負にならないと思っているのであれば、コードを修正する必要があります。序数が2のn乗であることが明らかならば、かわりにビットANDを使用することができます(x.hashCode()%n
の代わりにx.hashCode()&(n-1)
を使用します)。これは恐らく剰余計算よりも高速です。それ以外のケースでは剰余に対して絶対値を算出してください(Math.abs(x.hashCode()%n)。
b
を符号無しの0..255に変換したいのであれば、0xff & b
とします。
]]>
b[0]
の値が0xff
であってx
の値が0だとすると、((x << 8) | b[0])
は、0xff
が符号拡張で0xffffffff
となるので、結果は0xffffffff
となります。
とりわけ、以下のようにバイト配列に格納された値をint変数に取り出すコードはひどく間違っています。
int result = 0;
for(int i = 0; i < 4; i++)
result = ((result << 8) | b[i]);
かわりに以下のようなイディオムを使用することで正しく動作するようになります。
int result = 0;
for(int i = 0; i < 4; i++)
result = ((result << 8) | (b[i] &s; 0xff));
]]>
((event.detail & SWT.SELECTED) > 0). Using bit arithmetic and then comparing with the greater than operator can lead to unexpected results (of course depending on the value of SWT.SELECTED). If SWT.SELECTED is a negative number, this is a candidate for a bug. Even when SWT.SELECTED is not negative, it seems good practice to use '!= 0' instead of '> 0'.
Boris Bokowski
]]>((event.detail & SWT.SELECTED) > 0). Using bit arithmetic and then comparing with the greater than operator can lead to unexpected results (of course depending on the value of SWT.SELECTED). If SWT.SELECTED is a negative number, this is a candidate for a bug. Even when SWT.SELECTED is not negative, it seems good practice to use '!= 0' instead of '> 0'.
Boris Bokowski
]]>このバグの典型的なパターンとして、ビットセットのテストのために、論理積("&")を使うべきところを、間違って論理和("|")を使用してしまったケースが考えられます。
]]>java.util.concurrent.locks.Lock
オブジェクトで同期化を行っています。lock()
とunlock()
を替わりに使用すべきです。
]]>
明示的にStringBufferあるいはStringBuilder(J2SE 1.5から導入されます)を用いる事で、パフォーマンスを改善する事が出来ます。
例)
// 悪い例 String s = ""; for (int i = 0; i < field.length; ++i) { s = s + field[i]; } // 良い例 StringBuffer buf = new StringBuffer(); for (int i = 0; i < field.length; ++i) { buf.append(field[i]); } String s = buf.toString();]]>
myCollection.toArray(new Foo[myCollection.size()])
とする方が、効率の良いやり方です。もしも引数のサイズが、Collectionの中身を格納するのに十分な大きさであれば、そのまま、その配列に格納されて返送されます。さもなければリフレクションによって、新たに配列を生成しなければなりません。
]]>
public static junit.framework.Test suite()あるいは
public static junit.framework.TestSuite suite()と宣言する必要があります。 ]]>
[Bill Pugh]: すみません、私はこの警告の出力には強く反対で、あなたのコードは全く正しいと思います。ユーザのコードはequals()の実装がどうなっているかを意識すべきではなく、==でのインスタンス比較に依存すべきではありません。このようにしてしまうと、ライブラリ側でオブジェクトの同値性を制御することができなくなってしまいます。
]]>Foo.class
would force the static initializer
for Foo
to be executed, if it has not been executed already.
In Java 5 and later, it does not.
See Sun's article on Java SE compatibility for more details and examples, and suggestions on how to force class initialization in Java 5.
]]>if (x == Double.NaN)
)。しかしNaN
の定義上、全てのとり得る値はNaN
自身も含めてNan
とは等しくなりません。このためx == Double.NaN
は常にfalseとなります。値x
はNot A Numberであるかどうかを調べるにはDouble.isNaN(x)
(もしもxがfloatならばFloat.isNaN(x)
を代わりに使用してください。
]]>
if ( Math.abs(x - y) < .0000001 )
.
詳しくはJava言語仕様4.2.4.を参照してください。
]]>
メソッド | パラメータ |
---|---|
-任意- | |
0.0 か 1.0 | |
0.0 か 1.0 | |
0.0 か 1.0 | |
0.0 | |
0.0 か 1.0 | |
-任意- | |
0.0 | |
0.0 | |
0.0 か 1.0 | |
0.0 | |
-任意- | |
0.0 か 1.0 | |
0.0 か 1.0 | |
-任意- | |
-任意- | |
0.0 | |
0.0 | |
0.0 か 1.0 | |
0.0 | |
0.0 | |
0.0 か 1.0 | |
0.0 |
long convertDaysToMilliseconds(int days) { return 1000*3600*24*days; }
longの演算を用いて乗算を行うことで、オーバーフローしてしまう可能性を避けることができます。例えば、以下のように修正します。
long convertDaysToMilliseconds(int days) { return 1000L*3600*24*days; }
あるいは、
static final long MILLISECONDS_PER_DAY = 24L*3600*1000;
long convertDaysToMilliseconds(int days) { return days * MILLISECONDS_PER_DAY; }
]]>
]]>int x = 2; int y = 5; // 間違い: 結果は0.0となる double value1 = x / y; // 正しい: 結果は0.4となる double value2 = x / (double) y;
equals(Object o)
は引数o
の型に対して仮定を置いてはいけません。もしもo
が、this
と互換性の無い型であれば、単純にfalseを返さなければなりません。
]]>
List
、Set
、Map
)にキャストしています。そのオブジェクトは既にキャスト先の型でもあることに注意してください。また単にコレクションの中身を走査するだけなら、SetやListにキャストする必要はありません。
]]>
File.separator
が使用されています。これはWindowsプラットフォームでは正しく動作しません。File.separator
がバックスラッシュとなるためです。これは正規表現のエスケープキャラクタです。回避方法として、File.separatorChar=='\\' & "\\\\" : File.separator
を、File.separator
の替わりに使用する方法があります。
]]>
i++
)の後、ただちに内容を上書きしています。例えば、i = i++
は、インクリメントされた値を元の値で上書きします。
]]>
(low+high)/2
の代わりに(low+high) >>> 1
を使用します。
このバグは昔の二分探索、マージソートで多く見られます。Martin Buchholz がJDKの中に、このバグを発見、修正しています。またJoshua Blochがより一般化したバグパターンの話として紹介しています。
]]>new File("/home/dannyc/workspace/j2ee/src/share/com/sun/enterprise/deployment");
)
]]>
You may also experience serialization problems.
Using an instance field is recommended.
For more information on this see Sun Bug #6231579 and Sun Bug #6178997.
]]>For more information on this see Sun Bug #6231579 and Sun Bug #6178997.
]]>You may also experience serialization problems.
Using an instance field is recommended.
For more information on this see Sun Bug #6231579 and Sun Bug #6178997.
]]>For more information on this see Sun Bug #6231579 and Sun Bug #6178997.
]]>More precisely, a value annotated with a type qualifier specifying when=ALWAYS is guaranteed to reach a use or uses where the same type qualifier specifies when=NEVER.
For example, say that @NonNegative is a nickname for the type qualifier annotation @Negative(when=When.NEVER). The following code will generate this warning because the return statement requires a @NonNegative value, but receives one that is marked as @Negative.
]]>public @NonNegative Integer example(@Negative Integer value) { return value; }
More precisely, a value annotated with a type qualifier specifying when=NEVER is guaranteed to reach a use or uses where the same type qualifier specifies when=ALWAYS.
TODO: example
]]>For example, consider the following method:
public @Untainted Object mustReturnUntainted(Object unknown) { return unknown; }
The mustReturnUntainted
method is required to
return a value carrying the @Untainted annotation,
but a value not known to carry that annotation is returned.
TODO: example.
]]>The only situation in which opening a file in append mode and the writing an object output stream could work is if on reading the file you plan to open it in random access mode and seek to the byte offset where the append started.
TODO: example.
]]>this.getClass()
. If this class is subclassed,
subclasses will synchronize on the class object for the subclass, which isn't likely what was intended.
For example, consider this code from java.awt.Label:
private static final String base = "label"; private static int nameCounter = 0; String constructComponentName() { synchronized (getClass()) { return base + nameCounter++; } }
Subclasses of Label
won't synchronize on the same subclass, giving rise to a datarace.
Instead, this code should be synchronizing on Label.class
private static final String base = "label"; private static int nameCounter = 0; String constructComponentName() { synchronized (Label.class) { return base + nameCounter++; } }
Bug pattern contributed by Jason Mehrens
]]>