views:

198

answers:

7

Which do you prefer and why"

String myString = null;
if(someCondition)
   myString = "something";
else
   myString = "something else";

OR

String myString = "";
if(someCondition)
   myString = "something";
else
   myString = "something else";

I know that using the ternary (? :) operator is possible but I'd like to know about the above two.

+16  A: 

Neither. Instead, this:

String myString;
if (someCondition)
   myString = "something";
else
   myString = "something else";

In both of your alternatives, the variable is initialized with a value which will never ever be read. The fact that it's present at all is misleading.

I would actually use the conditional operator, of course - but barring that, the above is the better option.

Jon Skeet
I prefer this one as well. Why give myString a value that will just get replaced?
bwawok
+1 for conditional operator, it so shines in these kind of situations
Mahesh Velaga
It's not actually just a style preference for this one, it's really a better solution. The compiler has the ability to path check here and tell you if one of the paths fails to initialize the value before it's used. In the cases where you initialize it, you remove the compilers ability to help you with that.
Bill K
@Erick: No, see the comments on the question.
Mark Peters
+12  A: 

The idiomatic way is to use ternary/conditional operator (JLS 15.25):

String myString = (someCondition ? "something" : "something else");

But you can also do the more verbose if-else statement if you really feel you must:

final String myString;
if(someCondition) {
   myString = "something";
} else {
   myString = "something else";
}

Note that I've added final modifier in the above snippet. If you're planning on further reassignments to the variable, then of course it can't be final, so you can remove the modifier and of course the code would still work.


Why final?

The point of the final in the above snippet is to show that the if-else construct will assign to myString once and exactly once in all possible execution paths. That is the main idea of the proposed if-else solution: if you're going to assign a value to a local variable only once, even if it can be one of several possibilities, then make it final to enhance readability.

Contrast that with this "alternative" proposal for example:

// DON'T DO THIS! Example only!
String myString = "something else";
if (someCondition) myString = "something";

With this construct, you may be assigning to myString twice, thus you couldn't put final here even if there was no further reassignment. You also couldn't put final in either of the original = null; or = ""; proposals, and this is one of the main reasons why they're not recommendable.

There's no point in assigning a value to a variable if you're just going to overwrite it before you're going to use it. It hurts readability, and may potentially even hide bugs, e.g. when one execution path fails to overwrite this "initial" value.

References


Summary

  • Don't "initialize" a local variable just for the sake of doing it if you're going to overwrite it anyway
    • Let it be uninitialized, so that the compiler can help you identify a possible bug by pointing out any use of the variable while it's still uninitialized
    • If the code compiles, then the variable is assigned a "real" value at least once before all uses
  • If you don't need to reassign a local variable, make it final to enhance readability
    • final immediately assures readers that no further reassignments are possible
    • The compiler can help you prevent making the mistake of subsequent reassignment
    • If the code compiles, then the variable is assigned a "real" value exactly once before all uses
  • Generally speaking, you should let the compiler help you write the best, most readable code.
polygenelubricants
I don't think final has any bearing on whether a local variable needs an assignment, and your link seems to support that. Can you clarify what you mean?
Mark Peters
Great answer now, +1.
Mark Peters
Downvoter please explain because I revise my answers based on feedback.
polygenelubricants
A: 
String myString = "something else";
if(someCondition) myString = "something"; // (use curly braces if you prefer)
MCain
A: 

I prefer first one, because String myString = "" will create additional object in the pool

rachvela
Since it is a static String known at compile time, it will be a referenced to an interned string with a value of "".. so not an object, just a reference.
bwawok
yes, but this interned string have to be created in the string pool
rachvela
@rachvela: Once... at an insignificant cost, which would almost *certainly* occur anyway because most Java programs will touch a library *somewhere* which has an empty string.
Jon Skeet
A: 
String mystring = null;
mystring.length() 
// Cause error

Above will cause error due to null pointer.

string myString = new String();
myString.length()
// will not cause error

I like to use later, but I think it's personal preference.

Ankiov Spetsnaz
Creating a new string is pointless and wasteful... but using *any* value is somewhat pointless, as the value will immediately be discarded and a new one assigned.
Jon Skeet
That's true but totally irrelevant; he's not calling a function on the null pointer, he's reassigning it immediately
Michael Mrozek
+2  A: 

The initialization step is not necessary, and may confuse future readers.

My personal opinion is that this kind of variable should only be assigned once, hence it is a perfect candidate for the final keyword.

final String myString;
if (someCondition) {
   myString = "something";
} else {
   myString = "something else";
}

Note that the myString definition does not include an assignment (as this would prohibit later assignments) and that after the assignment it is read-only. This gives robust code and shows your intent more clearly.

Please also note that I believe in braces even for single lines. Probably a Perl habit, but if you don't, it will bite you someday.

Thorbjørn Ravn Andersen
A: 

How about this follwing code ,anyways he wants to set something.

String myString = (someCondition)  ? "something " : "else something";

or this

String myString = "else something"; 

if (someCondition)
   myString = "something";

in the above case , if you are 90% sure that someCondition is always true. otherwise unnecessary object creation in declaration.Expecting comments from Gurus.

Suresh S