Community
Participate
Working Groups
Wouldn't it be a better idea to replace EXCLUSION and INCLUSION static fields with an enum? Nothing prevents users from calling setAttribute() and getAttribute() methods by an arbitrary string parameter as key. Additionally, these string are compared against the the given key in getAttribute() and setAttribute() methods using the == operand. I reviewed the code carefully and, as far as I know, there is nothing wrong with this class. But it's not a good idea to use String objects as keys and compare them with the == operand instead of the equals() method.
I digged up the code and realized these fields (EXCLUSION and INCLUSION) are just used inside the FileScopeProblemPreference class. So, why they are public? Am I wrong about something?
I don't understand what is the bug about? It is ok to use ==, the only acceptable objects are the constants defined in this class. Nothing stops user to pass other stuff but it just won't work. They are public because they use by other package (as you mention FileScopeProblemPreference). I don't know why they implemented this way not using regular getter/setter for example, but this was copied from jdt.
(In reply to comment #2) > I don't understand what is the bug about? It is ok to use ==, > the only acceptable objects are the constants defined in this class. > Nothing stops user to pass other stuff but it just won't work. There is absolutely nothing wrong with this class. But if we use an enum, calls to setAttribute() and getAttribute() methods that use inappropriate parameters can be detected at compile time. > They are public > because they use by other package (as you mention FileScopeProblemPreference). You are right. Sorry, my bad! :( > I don't know why they implemented this way not using regular getter/setter for > example, but this was copied from jdt. Maybe we should ask the original developer why it is like this in jdt. BTW, this is one of the false positives reported by Findbugs.
