Expression might evaluate to null but is returned by the method declared as @NotNull

Question

After creating a RecyclerView adapter a warning about my view holder is returned. I've read this question and understand it a bit, but it's not clear as to what the viewHolder instance in return viewHolder; should be replaced with.

Expression might evaluate to null but is returned by the method declared as @NotNull

public class MyRVAapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> {
    private final static int TYPE_EXPANDABLE = 1, TYPE_NONEXPANDABLE = 2;
    private ArrayList callSMSFeed = new ArrayList();
    private Context context;

    public MyRVAapter(Context context){
        this.context = context;
    }

    public void setCallSMSFeed(List<Object> callSMSFeed){
        this.callSMSFeed = (ArrayList) callSMSFeed;
    }

  @Override
    public int getItemViewType(int position) {
        if (callSMSFeed.get(position) instanceof Phonecall) {
            return TYPE_EXPANDABLE;
        } else if (callSMSFeed.get(position) instanceof SMSmessage) {
            return TYPE_NONEXPANDABLE;
        }
        return -1;
    }

    @Override
    public void onBindViewHolder(@NonNull final RecyclerView.ViewHolder holder, final int position) {
        int viewType=holder.getItemViewType();
        switch (viewType){
            case TYPE_EXPANDABLE:
                Phonecall call = (Phonecall) callSMSFeed.get(position);
                ((CallViewHolder)holder).showCallDetails(call);
                break;
            case TYPE_NONEXPANDABLE:
                SMSmessage sms = (SMSmessage)callSMSFeed.get(position);
                ((SMSViewHolder)holder).showSmsDetails(sms);
                break;
        }
    }

    @Override
    public int getItemCount(){return callSMSFeed.size();}

    @NonNull
    @Override
    public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {


        int layout;

        RecyclerView.ViewHolder viewHolder;
        switch (viewType){
            case TYPE_EXPANDABLE:
                layout = R.layout.cardview_a;
                View callsView = LayoutInflater
                        .from(parent.getContext())
                        .inflate(layout, parent, false);
                viewHolder = new CallViewHolder(callsView);
                break;
            case TYPE_NONEXPANDABLE:
                layout = R.layout.cardview_b;
                View smsView = LayoutInflater
                        .from(parent.getContext())
                        .inflate(layout, parent, false);
                viewHolder = new SMSViewHolder(smsView);
                break;
            default:
                viewHolder = null;
                break;
        }
        return viewHolder;
    }
}

score 5 · Accepted Answer · answered Oct 10 '18 at 16:52

I like @TheWanderer's answer, but I wanted to add another in order to highlight something that I think will make your life easier.

Unlike ListView, RecyclerView doesn't care what values you return from getItemViewType(); as long as you return distinct values from this method, RecyclerView is happy. This allows you to use layout resource ids as return values, which means you never have to define your own constants!

@Override
public int getItemViewType(int position) {
    Object obj = callSMSFeed.get(position);

    if (obj instanceof Phonecall) {
        return R.layout.cardview_a;
    } else if (obj instanceof SMSmessage) {
        return R.layout.cardview_b;
    }

    throw new IllegalStateException("item at position " + position + " is not a Phonecall or SMSmessage: " + obj);
}

Here we return R.layout.cardview_a instead of TYPE_EXPANDABLE; both are ints under the hood, but now we can let the Resources framework take care of defining them for us.

We also throw an exception if we ever hit an Object that isn't a Phonecall or SMSmessage so that we know right away that there's a case we need to handle. This sort of exception should only be seen by developers making mistakes; the app won't ever crash on a user because you'll go ahead and fix the crash before you release the app.

@NonNull
@Override
public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
    LayoutInflater inflater = LayoutInflater.from(parent.getContext());
    View itemView = inflater.inflate(viewType, parent, false);

    switch (viewType) {
        case R.layout.cardview_a:
            return new CallViewHolder(itemView);

        case R.layout.cardview_b:
            return new SMSViewHolder(itemView);

        default:
            throw new IllegalArgumentException("unexpected viewType: " + viewType);
    }
}

Because we returned layout ids from getItemViewType(), we don't have to worry about translating from our own constants to R.layout values; we can just inflate the viewType directly. We still need the switch statement, though, to know which type of ViewHolder to return.

Again, we thrown an exception here in case we are ever passed a viewType we don't expect. This "won't happen", but if it ever does, it will be very clear where the problem is and easy for you to fix.

@Override
public void onBindViewHolder(@NonNull RecyclerView.ViewHolder holder, int position) {
    int viewType = holder.getItemViewType();

    switch (viewType) {
        case R.layout.cardview_a:
            Phonecall call = (Phonecall) callSMSFeed.get(position);
            ((CallViewHolder) holder).showCallDetails(call);
            break;

        case R.layout.cardview_b:
            SMSmessage sms = (SMSmessage) callSMSFeed.get(position);
            ((SMSViewHolder) holder).showSmsDetails(sms);
            break;

        default:
            throw new IllegalArgumentException("unexpected viewType: " + viewType);
    }
}

Here the only difference is replacing your constants with layout ids inside the switch. No big deal. And, of course, we crash if we get something we don't expect.

Note also that I've removed the final keyword from the params. Even though the compiler will happily let you add them, you shouldn't do it. Because of methods like notifyItemInserted(), it's possible for a ViewHolder's position to change over time without ever being re-bound. Dealing with this is outside the scope of your question, but it's something to point out.

TheWanderer · Answer 2 · 2018-10-10T16:38:07.087

@NonNull is just an IDE flag (and used for Kotlin cross-compatibility). It doesn't do much more than tell the IDE when it should and shouldn't warn you about potential null pointers.

Just because you declare it doesn't mean your method will return a non-null value. Of course, yours will just because of how it works, but the IDE isn't that smart. All it sees is a contradiction: you use the @NonNull parameter, but you also have a case where you return a null value.

~~The easiest way to fix this would be to just replace your default case with what's under TYPE_EXPANDABLE or TYPE_NONEXPANDABLE and remove the redundant case, ie:~~

EDIT: From Ben P's comment, it would also be a good idea to throw an Exception in your onCreateViewHolderMethod() if the current View type is unexpected.

switch (viewType){
    case TYPE_EXPANDABLE:
        layout = R.layout.cardview_a;
        View callsView = LayoutInflater
                .from(parent.getContext())
                .inflate(layout, parent, false);
        viewHolder = new CallViewHolder(callsView);
        break;
    case TYPE_NONEXPANDABLE:
        layout = R.layout.cardview_b;
        View smsView = LayoutInflater
                .from(parent.getContext())
                .inflate(layout, parent, false);
        viewHolder = new SMSViewHolder(smsView);
        break;
    default:
        throw IllegalArgumentException("Invalid View type: " + viewType);
}

~~It's not the most elegant, but as long as you know for sure that getItemViewType() can never return -1, you won't run into any weird behavior.~~

I would add one more thing to this, though, just so you can be sure you'll get what you want. Instead of this:

@Override
public int getItemViewType(int position) {
    if (callSMSFeed.get(position) instanceof Phonecall) {
        return TYPE_EXPANDABLE;
    } else if (callSMSFeed.get(position) instanceof SMSmessage) {
        return TYPE_NONEXPANDABLE;
    }
    return -1;
}

Try this:

@Override
public int getItemViewType(int position) {
    if (callSMSFeed.get(position) instanceof Phonecall) {
        return TYPE_EXPANDABLE;
    } else if (callSMSFeed.get(position) instanceof SMSmessage) {
        return TYPE_NONEXPANDABLE;
    }
    throw new IllegalArgumentException("Item at position " + position + " is not an instance of either Phonecall or SMSmessage");
}

Note: You can use whichever Exception type you think is fit for the case. I'm using IllegalArgumentException, Ben P (from comments) might use IllegalStateException... you could even just throw Exception.

I agree with most of this answer. I especially like throwing at the end of `getItemViewType()` (though I would personally choose `IllegalStateException` or `AssertionError`)... but I would say that you should _also_ throw in `onCreateViewHolder()` if the `viewType` argument is something you don't expect. Much better for your app to crash if you get passed some third option you don't expect (maybe you later change `getItemViewType()` but forget to change `onCreateViewHolder()`) than to return the wrong `ViewHolder`. — Ben P., Oct 10 '18 at 16:30

score 0 · Answer 3 · answered Oct 10 '18 at 16:22

You have this

@NonNull

before your onCreateViewHolder. The warning message is clearly telling you that you need to make sure you return something which is not null. In the case of default, viewHolder is null indeed, you might want to return a valid value, or throw an exception in that case.

score 0 · Answer 4 · answered Oct 10 '18 at 16:32

you have this warning because the method

public RecyclerView.ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {

must not return null, but on your default case you are returning null

default:
                viewHolder = null;
                break;

On production, the default case will lead to a crash, and that is why the warning is rised. You have to throw a runtime Exception or return a default non null view.

Expression might evaluate to null but is returned by the method declared as @NotNull

4 Answers4