• Post Reply Bookmark Topic Watch Topic
  • New Topic

findbugs and pmd disagree  RSS feed

 
Jim Venolia
Ranch Hand
Posts: 312
2
Chrome Linux VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I had a line of code "String longString = new String("");". Findbugs said I didn't need the 'new String' part, so I changed it to 'String longString = "";' Now pmd is saying "An operation on an Immutable object (String, BigDecimal or BigInteger) wont change the object itself".

IMHO, the line was correct with the 'new String("");', but I'm a n00b and thought I'd ask here.
 
Stephan van Hulst
Saloon Keeper
Posts: 7993
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't understand the pmd warning. It's probably an oversight.

you should never use the String constructor with a String literal. A string literal is already an object, and you're just making a copy of that object when you invoke the constructor, which is useless and confusing. Also, it prevents your program from using the String pool effectively.
 
Jim Venolia
Ranch Hand
Posts: 312
2
Chrome Linux VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Both findbugs and pmd are reminding me of my early encounters with lint: useless until you tell it what to ignore. Right now, in my 700+ line program, I've got 84 "Useless parenthesis" messages. I use parens to make the code readable, not because I don't understand the precedence rules.

Problem is, I was a C expert before I ever ran lint. I'm a java n00b who doesn't know which lines are useless and which are useful.

I really like the "possible God class" followed by a cryptic bunch of stuff. Makes me feel like a super-java dude
 
Jeanne Boyarsky
author & internet detective
Marshal
Posts: 37513
554
Eclipse IDE Java VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
FindBugs is right. What line of code is triggering the immutable finding? I suspect there is another line that isn't the assignment.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jim Venolia wrote:I really like the "possible God class" followed by a cryptic bunch of stuff. Makes me feel like a super-java dude

Sorry to bring you back down to earth but the "God Class" is an anti-pattern
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jim Venolia wrote:IMHO, the line was correct with the 'new String("");', but I'm a n00b and thought I'd ask here.

No.

Class String is immutable. That means that once a String object has been created, it cannot be modified. Class String has no methods to modify the content of a String object - if you look carefully at the API documentation, you'll notice that all of its methods for manipulating strings will return a new String object, instead of modifying the String object that you call the method on.

Immutability has a number of advantages. It makes it easier to reason about in a multi-threaded environment - the main thing that makes multi-threaded programming hard is the fact that multiple threads can modify the same data, so you need to be very careful about synchronizing threads. It also enables Java to do certain optimizations, such as the String pool.

When you use the same string literal multiple times in a Java program, for example you have "hello" in the program multiple times, then Java will create just one String object that is shared by all the places where you use "hello" - it is not going to create a new String every time you use "hello" in the program.

When you are explicitly creating a new String object from the literal, as you are doing: new String("") then you are counteracting this optimization. You are explicitly creating a new String object which will contain a copy of the content of the string literal (which is in this case, the empty string).

So, doing new String("") is incorrect in the sense that it unnecessarily creates a new String object, counteracting Java's string sharing mechanism.

Never call new String(...) with a string literal; just use the string literal directly: String longString = "";
 
Don't get me started about those stupid light bulbs.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!