2
\$\begingroup\$

I want to replace an object of a List with another one. To find the item to replace I need to check the field id of the object to find what item is the same.

public void changeItemInformation(@NonNull Suggestion suggestion) {
    if (mSuggestions != null) {
        int itemChanged = -1;
        int index = -1;
        for (Suggestion suggestionItem : mSuggestions) {
            index++;
            if(suggestionItem.getId() == suggestion.getId()){
                itemChanged = index;
                break;
            }
        }
        if (itemChanged != -1) {
            mSuggestions.set(itemChanged, suggestion);
            notifyItemChanged(itemChanged);
        }
    }
}
  • mSuggestions is the list of Suggestions.
  • suggestion is the new item to replace the old one.
  • notifyItemChanged() is an Android method to notify a RecyclerView that an element changed.

Is there a better way to do this? There aren't any perform problems, the list always has a maximum of 100 elements.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I don't like your itemChanged and index variables.
If you iterate over a list and keep track of the index anyway, why don't you use an ordinary for-loop?
In addition to that, you also just copy the value of your index variable and break out of the loop; you could easily merge all that to one loop with one condition.
This:

    int itemChanged = -1;
    int index = -1;
    for (Suggestion suggestionItem : mSuggestions) {
        index++;
        if(suggestionItem.getId() == suggestion.getId()){
            itemChanged = index;
            break;
        }
    }
    if (itemChanged != -1) {
        mSuggestions.set(itemChanged, suggestion);
        notifyItemChanged(itemChanged);
    }

would become this:

for (int index = 0; index < mSuggestions.size(); index++) {
    if(mSuggestions.get(index).getId() == suggestion.getId()){
        mSuggestions.set(index, suggestion);
        notifyItemChanged(index);
        break;
    }
}

You don't even need to check if there is an item to replace, because if the condition is never met, it's never going to execute the code inside of it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.