Calling equals on string literal

Just to ensure there is a fully balanced set of answers to this question I would like to post a different opinion.

I think this mechanism is foolish.

If you have a null you should know as soon as it happens - hiding it will just postpone its discovery. If the null is not an exception, replace it with something else.

Taking this approach will fortify your code with what is called defensive programming where your mistakes are discovered as soon as possible rather than covered up until everything falls apart.

In summary - NullPointerException is your friend. You should use it to find mistakes in your code. It is very easy to use a Empty object such as Collections.emptySet() once you have determined that the null is not an exception.

Using the Yoda technique out of habit will inevitably hide errors you do not mean to hide. Not using it will expose errors much earlier. To me that is sufficient argument to not use it - ever.

To me - using

if(saving != null && saving.equals("true")){

means that I actually want to allow savings to be null and it is an acceptable situation - using

if("true".equals(saving)){

merely hides that deliberate choice in a way that could become a bad habit.

This is safe - and as you have seen, a good way of avoiding null pointers.

You mention the use of new for Strings. Many java static code analysis tools will recommend always using literals over new String("foo");.

Edit:

If you wanted, you could even just use:

if (Boolean.valueOf(saving)) {
    ...
}

According to the docs, passing null will return false.

It's called yoda conditions (putting a constant before a variable in a comparison) can be considered like bad practise, maybe for someone much less readable (like me).

But sometimes using yoda condition makes the code more "comprehensible" -> you don't need to put extra null check in front of it and it efficiently differentiates block of code from case where null is strongly forbidden.

The format of putting the literal first avoids the potential NPE and will always be false for "literal".equals(null).

This is because a "literal" expression always evaluates to a String object (and is thus never null) and String.equals(obj) checks to see if the other object is a null (via asinstanceof). An object is an object - no need to worry bout the "heap".

It's good practice to have a null-guard in an equals implementation as null.equals(null) is not permissible: "The equals method implements an equivalence relation on non-null object references"

Here is the String.equals the source (for OpenJDK):

public boolean equals(Object anObject) {
    if (this == anObject) {
        return true;
    }
    if (anObject instanceof String) {
        // ..
    }
    return false;
}

if("true".equals(saving)){
   // do something
}

This is very safe and good practice. String "true" will never be null. So you will never compare your String to null. This piece of code is perfectly fine

if ("true".equals(saving))

is perfectly "save" and recommended by dummy product as SonarLint and any other advisors that establish this "well" thinked rule that your chief request that you follow.

I say stupid and my explanation is the following.

What happens if you have following code

if (!saving.equals("true"))
    {
    if (saving.length() > 10)
        {
        // do something
        }
    }

You can use following advised code

if (!"true".equals(saving))
    {
    if (saving.length() > 10)
        {
        // do something
        }
    }

But when your code is deployed on production, the program will crash on length() function if saving variable has not been initialized and contains NULL.

You have changing your code to avoid NullPointerException on equals() function but your program will return another NullPointerException on length() function !

The good reaction is to stop to use String literal tip and to analyze code to find a perfect solution as in the following example.

if (saving != null)
    {
    if (!saving.equals("true"))
        {
        if (saving.length() > 10)
            {
            // do something
            }
        }
    }

or

if (saving == null) saving = "";

if (!saving.equals("true"))
    {
    if (saving.length() > 10)
        {
        // do something
        }
    }

Is is perhaps verbose but if you will use Java for programming it is the price to pay.

Using String literal equals() method to avoid NullPointerException has the following disadvantages

1. the solution is tricky
2. the code is not naturally readable for human
3. equality will not crash but another part of program will continue to crash
4. code become quickly difficult to read if this tips is used for lower() and greater() functions (example: if ('USA'.greater(sCode)))
5. give a false impression that the code is safe