Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 317361

Summary:	Enhancement request in org.eclipse.cdt.codan.core.param.FileScopeProblemPreference
Product:	[Tools] CDT	Reporter:	Meisam <meisam.fathi>
Component:	cdt-codan	Assignee:	Project Inbox <cdt-core-inbox>
Status:	RESOLVED WONTFIX	QA Contact:	Elena Laskavaia <elaskavaia.cdt>
Severity:	minor
Priority:	P3	CC:	cdtdoug
Version:	7.0
Target Milestone:	---
Hardware:	PC
OS:	All
Whiteboard:

Description Meisam

2010-06-19 11:57:14 EDT

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.

Comment 1 Meisam

2010-06-19 12:07:52 EDT

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?

Comment 2 Elena Laskavaia

2010-06-19 13:32:59 EDT

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.

Comment 3 Meisam

2010-06-19 23:38:41 EDT

(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.