Is Null Check Needed Before Calling Instanceof

Is null check needed before calling instanceof?

No, a null check is not needed before using instanceof.

The expression x instanceof SomeClass is false if x is null.

The Java 11 Language Specification expresses this concisely in section 15.20.2, "Type comparison operator instanceof". (Java 17 expresses this less concisely, after the introduction of instanceof patternmatching.)

"At run time, the result of the
instanceof operator is true if the
value of the RelationalExpression is
not null and the reference could be
cast to the ReferenceType
without raising a ClassCastException.
Otherwise the result is false."

So if the operand is null, the result is false.

Why or why not use instanceof to check if not null

That depends on what you are actually checking

• If you just want to know whether the reference is null, use x == null and nothing else

• If you want to know whether a particular reference is not null and points to an instance of type Foo you may use x instanceof Foo as it implies being not null

  But if the compile-time type of the reference is already Foo, you know that the instance is of type Foo when it is non-null, therefore the first bullet applies in this case; you just want to test for null. This is the case in your updated question’s example, the reference already has the compile-time type View.

• If you want to know whether a type cast (Foo)x will succeed, you may use x == null || x instanceof Foo as the type cast will also succeed when x is null (however, you should think twice whether you really want to accept nulls, even if the type cast will be successful)

PMD: how to deal with no need to check null before instanceof

If you can't use the fixed version of PMD, add a temporary variable

... principal = null;
if (SecurityContextHolder.getContext().getAuthentication() != null)
   principal = SecurityContextHolder.getContext().getAuthentication()
                    .getPrincipal();
if (principal instanceof User) ...

Null instanceof Object

It probably just means that the IDEA code analyzier is not writtent to support this "use case". The analyzer is probably (intentionally) coded to detect the use of variables that have previously been assigned to null, since this is a quite common mistake to make while coding, but the null instance of Object case it not very common as very few programmers would write this while actually meaning something else.

There are many constructs like these that can not be correctly detected by static code analyzers like Sonar, Eclipse or IDEA. I would not worry much about it, just be aware that the IDE can not spot every mistake you make in your code.

Null Pointer Exception when calling variable from another class

I pasted your code and ran it locally. When prompted, I entered "100". Here's the entire output:

Input price of car
100
0.0
0.0
Exception in thread "main" java.lang.NullPointerException: Cannot read field "price" because "this.car" is null
    at ComprehensivePolicy.calcPayment(main.java:102)
    at main.main(main.java:17)

Why did that fail? Because Car car; is declared in the abstract class, and code tries to use it (car.price) without first assigning a value or creating a Car object. So, car is null, and calling anything on it (like "car.price") will throw NullPointerException.

Here's a very simple fix to get it working: right after creating a new "ThirdPartyPolicy", assign the car you created a few lines earlier:

Car c = new Car("model", 1998, 123);
...
ThirdPartyPolicy tp = new ThirdPartyPolicy("hello");
tp.car = c;

After doing that, you'll see another exception which looks identical to the first. It's due to the shared code in the abstract class, again with null "car". Here's a similar simple fix to get it working.

ComprehensivePolicy comp = new ComprehensivePolicy(2, 31);
comp.car = c;

After adding both of those lines, the program runs without error.

Here's a cleaner way of making things work.

1. Modify the "ThirdPartyPolicy" constructor to require a Car parameter, and save the reference (this.car = car):
   

   public ThirdPartyPolicy(Car car, String comments) {
       this.comments = comments;
       this.car = car;
   }
   

2. Similar change for "ComprehensivePolicy":
   

   public ComprehensivePolicy(Car car, int level, int driverage) {
       this.car = car;
       this.level = level;
       this.driverage = driverage;
   }
   

3. Pass the "car" reference to each of the constructors:
   

   Car car = new Car("model", 1998, 123);
   ThirdPartyPolicy tp = new ThirdPartyPolicy(car, "hello");
   ComprehensivePolicy comp = new ComprehensivePolicy(car, 2, 31);
   

Error in using instanceOf operator in Java

There is a number of things that you can improve.

First, the instanceof keyword is lowercase.

Second, if you override equals(Object), you MUST also override hashCode(), as mentioned in this answer. This answer also contains excellent examples of how to implement both equals and hashCode.

Third, your implementation allows name to be null, in which case this.getName() is null and your check

this.getName().equals(input.getName()) 

throws a NullPointerException. I suggest you follow the more robust practice in the answer or this tutorial, or make sure that name can never be null, by checking it in the constructor and setName.

Additional improvements and remarks

If there is a field ID, I assume that it is there to identify the employee. In that case, it must be included in the equals check.

If there is an 'ID`, why also check the name and the salary? Does changing the salary also mean that it becomes a different Employee? Same if I correct the name of the Employee?

The initialisation of ID is not thread-safe, and can lead to different Employees having the same ID. Consider doing:

import java.util.concurrent.atomic.AtomicInteger;
///
private static AtomicInteger lastID = new AtomicInteger();

And in the constructor just do

this.ID = lastID.getAndIncrement();

which makes the following line unnecessary:

Employee.lastID=Employee.lastID+1;

Another thing is that the method raise also allows the salary to be set lower than its previous value. I would either check for that, or rename raise to setSalary.

Given all that, it would lead to something like:

import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;

public class Employee {
    private static AtomicInteger lastID = new AtomicInteger();
    private String name;
    private final int ID;
    private double salary;

    public Employee(String name, double salary) {
        this.name = name;
        ID = lastID.getAndIncrement();
        this.salary = salary;
    }

    public String getName() { return name; }

    public int getID() { return ID; }

    public double getSalary() { return salary; }

    public void setName(String newName) { name = newName; }

    public void setSalary(double newSalary) { salary = newSalary; }

    @Override
    public String toString() {
        return "Name: " + getName() + "\nID: " + getID() + "\nSalary: " + getSalary();
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof Employee)) return false;
        Employee employee = (Employee) o;
        return ID == employee.ID;
    }

    @Override
    public int hashCode() {
        return Objects.hash(ID);
    }
}

---

Related Topics