Should I pre-initialize a variable that is overwritten in multiple branches?

Question

There is a method:

private String myMethod(String gender)
{
    String newString = "";
    if(gender.equals("a"))
        newString = internal.getValue();
    else
        newString = external.getValue();

    return newString;
}

I refactored everything, but with one small change: String newString; instead of: String newString = "";

Does this refactor improve the code? I know that String is null when we don't initialize it, but in this example it always will have value a from if or else. Does this refactor change anything?

Answer 1

To answer the direct question: there's no need to assign a value initially here; all branches of the code's execution will pan out to giving newString a value. Thus you don't need to initialize it at all. Otherwise, I would initialize to whatever you would want as a "default" value.

Instead of two returns or a branching statement to assign a variable, I would just return with a ternary:

private String myMethod(String gender) {
    return gender.equals("a")
            ? internal.getValue()
            : external.getValue();
}

Answer 2

Is it better to initialize String or to leave it as null?

Your premise is flawed: not initializing the String doesn't mean its value is null.

You are not allowed to use a local variable before it has been assigned, in order to avoid you accidentally using a value you didn't intend. As such, the value isn't "null", it's undefined (*).

This is called definite assignment checking, and is there to prevent certain types of bug. If you give the variable a value you don't need to give it, you disable this check, and so are open to the bugs the compiler was trying to protect you from.

For example, if the code looked like this:

private String myMethod(String gender)
{
    String newString = "";
    if(gender.equals("a"))
        newString = internal.getValue();
    else if (gender.equals("b");
        newString = external.getValue();
    // Oops! meant to check if gender.equals("c")

    return newString;
}

you might have a bug, because there is a missing case that you haven't checked.

If you had explicitly assigned null to the variable, you would have much the same issue; but now your method would return null, and so possibly cause an NPE in the calling code.

If you had omitted the = "", the compiler would stop you using newString in the return.

(Assigning and reassigning the variable also means the variable would not be effectively final, so you would be unable to use it inside a lambda or anonymous class).

---

(*) This only applies to local variables, and final member/static variables. Class members do not have to be definitely assigned before use if they are not final, which is a rich seam for bugs, and a good reason to make class members final wherever possible. And, technically, final members are initialized to their type's default value first, so you can actually read them as null before they are initialized.

Answer 3

It's best to only initialize a String (or anything else) if there is a scenario in which the initial value is used.

In your case you have assigned newString to a string literal that serves no purpose but to confuse the reader.

It should be evident that the performance and functionality will not change in any relavent way.