New Intention: Integer.valueOf instead of Integer.getInteger
This is the third time in so many months where I had a program bug:
logger.warning("getCommentForMenuItem("menuText')');
int idEnd = menuText.indexOf(':');
int idStart = menuText.lastIndexOf(' ', idEnd);
String idString = menuText.substring(idStart + 1, idEnd);
Integer key = Integer.getInteger(idString);
Comment comment;
if (displayAsCluster && cluster != null) {
comment = (Comment) clusterComments.get(key);
} else {
comment = (Comment) nodeComments.get(key);
}
return comment;
Which, given the menu item
Edit comment 273: "Pathway test comment"
is supposed to hunt up item corresponding to the Integer key value 273
in the clusterComments HashMap.
It, of course, does not, because the proper code would have
Integer.valueOf rather than Integer.getInteger.
Any chance of seeing this as a warning?
Scott
Please sign in to leave a comment.
How would Idea know looking up that value as a system property was not the intent?
Yeah, that's reasonable. The best thing would be to please find all the back-alley ways of getting stuff out of System properties (I knew about Boolean.getBoolean(), but had never heard of Integer.getInteger()) and write 'em up in a JIRA item. Once I get a spec, this'll be pretty easy, so it won't take long.
--Dave Griffith
Most inspections are for constructs that theoretically could be intended, but are in fact more likely to be bugs or design flaws. Using these frankly weird system property access methods is a great example of that. It's extremely fortunate that so many bugs come in stereotyped patterns. Unfortunately, the great mass of "I didn't really understand the requirements" bugs aren't stereotyped that way.
Hoever, in cases like this, I am less likely to provide a quickfix, or at least limit the quickfix to interactive use only. There's too much danger of actually screwing up programs to allow this to be fixed in batch.
--Dave Griffith.
In article <26823611.1116046582067.JavaMail.itn@is.intellij.net>,
Kirk Woll <kirk@digimax.com> wrote:
Like many of the warnings, it can be turned off on an individual or
global basis. What makes it dangerous is the class it is in. Were I
doing something with a Properties object, I would not have had any
expectations on it. In Integer, though, it has a misleading name, at
least IMO.
Scott
In article <621356.1116071218559.JavaMail.itn@is.intellij.net>,
Dave Griffith <dave.griffith@cnn.com> wrote:
Hi, Dave.
Ok, it took a while, but I finally got back from WWDC and my neice's
graduation, and filed it. It turns out that the only ones I found were
Boolean.getBoolean, Integer,getInteger, and Long.getLong.
http://www.jetbrains.net/jira/browse/IDEA-2981
I added a request for a intention to either turn it into getValue or
into System.getProperty with decode, etc. Just the first would solve
this problem in the majority of cases.
If the more complicated intention seems interesting, I can flesh it out
a bit. The inspection would go a long way towards finding and fixing it.
Scott
--
Scott Ellsworth
scott@alodar.nospam.com
Java and database consulting for the life sciences
I missed the Long.getLong(), but have already implemented the check for the others as "Archaic System properties access". I'll add Long.getLong() on the train tommorrow, and it'll be in the next EAP. Now that I can put two quickfixes on an inspection, all put fixes fo either getValue or System.getProperty with appropriate decode method. Both will be pretty easy to implement.
--Dave Griffith
In article <8558772.1119568396909.JavaMail.itn@is.intellij.net>,
Dave Griffith <dave.griffith@cnn.com> wrote:
Very, very keen. Thanks, Dave.
Scott
--
Scott Ellsworth
scott@alodar.nospam.com
Java and database consulting for the life sciences