Reputation: 8183
I am using a string as a lock and so want to ensure the object is a new instance. FindBugs complains because it's generally more efficient to define the string directly (with double quotes). My code looks like:
/** A lock for the list of inputs. */
@edu.umd.cs.findbugs.annotations.SuppressWarnings("DM_STRING_CTOR")
//We want a new String object here as this is a lock.
private final Object inputListLock = new String("inputListLock");
Am I doing something wrong here? The Eclipse FindBugs plugin is still reporting this as a problem:
Pattern id: DM_STRING_CTOR, type: Dm, category: PERFORMANCE Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter. Just use the argument String directly.
Upvotes: 3
Views: 5144
Reputation: 718788
The normal idiom is to do this:
private final Object inputListLock = new Object();
which saves space (relative to new String("someLock")
) and gets rid of the pesky PMD warning. But if you really want the lock to be a String, there are other ways to create a copy of a String that PMD is unlikely to object to; e.g.
private final Object inputListLock = "some".concat("Lock");
(Note that "someLock".concat("")
doesn't actually create a new String!)
Upvotes: 2
Reputation: 8183
Ok, so although both the other answers were interesting and useful (+1 for both), I didn't end up changing the code and I'm going to accept my own answer. To satisfy FindBugs I moved the annotation from the member variable to the surrounding class.
I've looked for some time but I haven't found any information suggesting that the SuppressWarnings may only be applied to classes and methods. Neither have I found any examples of it being applied to member variables. So though this solution works I don't know that it's the 'right' solution (maybe there's still something wrong with my FindBugs/Eclipse setup for example).
Upvotes: 1
Reputation: 54306
Why not just declare the lock object as a new Object? You don't need to make it a String, since you don't do anything that requires the String-ness of the lock, and presumable you don't use it for anything other than locking.
Without seeing the rest of your code I can hazard a guess that you're locking on access to a list of some kind. You could use the list itself as the lock object. If it's private then there is no chance that someone else will cause a deadlock.
Upvotes: 5