Fix placeholder position check (#1216)

* Fix placeholder position check

Because of the missed negation `placeholderPosition` could actually point to a different comment. As a result loaded comments would be displayed at a wrong position which could result in duplicated comments.

* Extract placeholder search code

Simple refactoring. The only notable change is that now technically `placeholderPosition` can be -1. In practice that should never happen, but I added checks anyways

1 files changed, 67 insertions, 78 deletions

diff --git a/app/src/main/java/ml/docilealligator/infinityforreddit/adapters/CommentsRecyclerViewAdapter.java b/app/src/main/java/ml/docilealligator/infinityforreddit/adapters/CommentsRecyclerViewAdapter.java
index a1fb48c4..1b534a0c 100644
--- a/app/src/main/java/ml/docilealligator/infinityforreddit/adapters/CommentsRecyclerViewAdapter.java
+++ b/app/src/main/java/ml/docilealligator/infinityforreddit/adapters/CommentsRecyclerViewAdapter.java
@@ -645,53 +645,39 @@ public class CommentsRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerVi
                                                     mVisibleComments.get(parentPosition).getChildren().get(mVisibleComments.get(parentPosition).getChildren().size() - 1)
                                                             .setLoadMoreChildrenFailed(false);
 
-                                                    int placeholderPosition = commentPosition;
-                                                    if (mVisibleComments.get(commentPosition).getFullName().equals(parentComment.getFullName())) {
-                                                        for (int i = parentPosition + 1; i < mVisibleComments.size(); i++) {
-                                                            if (mVisibleComments.get(i).getFullName().equals(parentComment.getFullName())) {
-                                                                placeholderPosition = i;
-                                                                break;
-                                                            }
-                                                        }
-                                                    }
-
-                                                    mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
-                                                    mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(false);
-                                                    ((LoadMoreChildCommentsViewHolder) holder).placeholderTextView.setText(R.string.comment_load_more_comments);
+                                                    int placeholderPosition = findLoadMoreCommentsPlaceholderPosition(parentComment.getFullName(), commentPosition);
+                                                    if (placeholderPosition != -1) {
+                                                        mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
+                                                        mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(false);
+                                                        ((LoadMoreChildCommentsViewHolder) holder).placeholderTextView.setText(R.string.comment_load_more_comments);
 
-                                                    mVisibleComments.addAll(placeholderPosition, expandedComments);
-                                                    if (mIsSingleCommentThreadMode) {
-                                                        notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
-                                                    } else {
-                                                        notifyItemRangeInserted(placeholderPosition, expandedComments.size());
+                                                        mVisibleComments.addAll(placeholderPosition, expandedComments);
+                                                        if (mIsSingleCommentThreadMode) {
+                                                            notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
+                                                        } else {
+                                                            notifyItemRangeInserted(placeholderPosition, expandedComments.size());
+                                                        }
                                                     }
                                                 } else {
                                                     mVisibleComments.get(parentPosition).getChildren()
                                                             .remove(mVisibleComments.get(parentPosition).getChildren().size() - 1);
                                                     mVisibleComments.get(parentPosition).removeMoreChildrenIds();
 
-                                                    int placeholderPosition = commentPosition;
-                                                    if (mVisibleComments.get(commentPosition).getFullName().equals(parentComment.getFullName())) {
-                                                        for (int i = parentPosition + 1; i < mVisibleComments.size(); i++) {
-                                                            if (mVisibleComments.get(i).getFullName().equals(parentComment.getFullName())) {
-                                                                placeholderPosition = i;
-                                                                break;
-                                                            }
+                                                    int placeholderPosition = findLoadMoreCommentsPlaceholderPosition(parentComment.getFullName(), commentPosition);
+                                                    if (placeholderPosition != -1) {
+                                                        mVisibleComments.remove(placeholderPosition);
+                                                        if (mIsSingleCommentThreadMode) {
+                                                            notifyItemRemoved(placeholderPosition + 1);
+                                                        } else {
+                                                            notifyItemRemoved(placeholderPosition);
                                                         }
-                                                    }
-
-                                                    mVisibleComments.remove(placeholderPosition);
-                                                    if (mIsSingleCommentThreadMode) {
-                                                        notifyItemRemoved(placeholderPosition + 1);
-                                                    } else {
-                                                        notifyItemRemoved(placeholderPosition);
-                                                    }
 
-                                                    mVisibleComments.addAll(placeholderPosition, expandedComments);
-                                                    if (mIsSingleCommentThreadMode) {
-                                                        notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
-                                                    } else {
-                                                        notifyItemRangeInserted(placeholderPosition, expandedComments.size());
+                                                        mVisibleComments.addAll(placeholderPosition, expandedComments);
+                                                        if (mIsSingleCommentThreadMode) {
+                                                            notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
+                                                        } else {
+                                                            notifyItemRangeInserted(placeholderPosition, expandedComments.size());
+                                                        }
                                                     }
                                                 }
                                             } else {
@@ -712,27 +698,21 @@ public class CommentsRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerVi
                                             for (int i = 0; i < mVisibleComments.size(); i++) {
                                                 if (mVisibleComments.get(i).getFullName().equals(parentComment.getFullName())) {
                                                     if (mVisibleComments.get(i).isExpanded()) {
-                                                        int placeholderPosition = i + mVisibleComments.get(i).getChildren().size();
-
-                                                        if (!mVisibleComments.get(i).getFullName()
-                                                                .equals(mVisibleComments.get(placeholderPosition).getFullName())) {
-                                                            for (int j = i + 1; j < mVisibleComments.size(); j++) {
-                                                                if (mVisibleComments.get(j).getFullName().equals(mVisibleComments.get(i).getFullName())) {
-                                                                    placeholderPosition = j;
-                                                                }
+                                                        int placeholderPositionHint = i + mVisibleComments.get(i).getChildren().size();
+                                                        int placeholderPosition = findLoadMoreCommentsPlaceholderPosition(parentComment.getFullName(), placeholderPositionHint);
+
+                                                        if (placeholderPosition != -1) {
+                                                            mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
+                                                            mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(false);
+                                                            ((LoadMoreChildCommentsViewHolder) holder).placeholderTextView.setText(R.string.comment_load_more_comments);
+
+                                                            mVisibleComments.addAll(placeholderPosition, expandedComments);
+                                                            if (mIsSingleCommentThreadMode) {
+                                                                notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
+                                                            } else {
+                                                                notifyItemRangeInserted(placeholderPosition, expandedComments.size());
                                                             }
                                                         }
-
-                                                        mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
-                                                        mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(false);
-                                                        ((LoadMoreChildCommentsViewHolder) holder).placeholderTextView.setText(R.string.comment_load_more_comments);
-
-                                                        mVisibleComments.addAll(placeholderPosition, expandedComments);
-                                                        if (mIsSingleCommentThreadMode) {
-                                                            notifyItemRangeInserted(placeholderPosition + 1, expandedComments.size());
-                                                        } else {
-                                                            notifyItemRangeInserted(placeholderPosition, expandedComments.size());
-                                                        }
                                                     }
 
                                                     mVisibleComments.get(i).getChildren().get(mVisibleComments.get(i).getChildren().size() - 1)
@@ -758,17 +738,9 @@ public class CommentsRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerVi
                                                 && parentComment.getFullName().equals(mVisibleComments.get(parentPosition).getFullName())) {
                                             if (mVisibleComments.get(parentPosition).isExpanded()) {
                                                 int commentPosition = mIsSingleCommentThreadMode ? holder.getBindingAdapterPosition() - 1 : holder.getBindingAdapterPosition();
-                                                int placeholderPosition = commentPosition;
-                                                if (commentPosition >= mVisibleComments.size() || commentPosition < 0 || !mVisibleComments.get(commentPosition).getFullName().equals(parentComment.getFullName())) {
-                                                    for (int i = parentPosition + 1; i < mVisibleComments.size(); i++) {
-                                                        if (mVisibleComments.get(i).getFullName().equals(parentComment.getFullName())) {
-                                                            placeholderPosition = i;
-                                                            break;
-                                                        }
-                                                    }
-                                                }
+                                                int placeholderPosition = findLoadMoreCommentsPlaceholderPosition(parentComment.getFullName(), commentPosition);
 
-                                                if (placeholderPosition >= mVisibleComments.size() || placeholderPosition < 0) {
+                                                if (placeholderPosition != -1) {
                                                     mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
                                                     mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(true);
                                                 }
@@ -783,18 +755,13 @@ public class CommentsRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerVi
                                             for (int i = 0; i < mVisibleComments.size(); i++) {
                                                 if (mVisibleComments.get(i).getFullName().equals(parentComment.getFullName())) {
                                                     if (mVisibleComments.get(i).isExpanded()) {
-                                                        int placeholderPosition = i + mVisibleComments.get(i).getChildren().size();
-                                                        if (!mVisibleComments.get(placeholderPosition).getFullName().equals(mVisibleComments.get(i).getFullName())) {
-                                                            for (int j = i + 1; j < mVisibleComments.size(); j++) {
-                                                                if (mVisibleComments.get(j).getFullName().equals(mVisibleComments.get(i).getFullName())) {
-                                                                    placeholderPosition = j;
-                                                                    break;
-                                                                }
-                                                            }
-                                                        }
+                                                        int placeholderPositionHint = i + mVisibleComments.get(i).getChildren().size();
+                                                        int placeholderPosition = findLoadMoreCommentsPlaceholderPosition(parentComment.getFullName(), placeholderPositionHint);
 
-                                                        mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
-                                                        mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(true);
+                                                        if (placeholderPosition != -1) {
+                                                            mVisibleComments.get(placeholderPosition).setLoadingMoreChildren(false);
+                                                            mVisibleComments.get(placeholderPosition).setLoadMoreChildrenFailed(true);
+                                                        }
                                                         ((LoadMoreChildCommentsViewHolder) holder).placeholderTextView.setText(R.string.comment_load_more_comments_failed);
                                                     }
 
@@ -836,6 +803,28 @@ public class CommentsRecyclerViewAdapter extends RecyclerView.Adapter<RecyclerVi
         return -1;
     }
 
+    /**
+     * Find position of comment with given {@code fullName} and
+     * {@link Comment#PLACEHOLDER_LOAD_MORE_COMMENTS} placeholder type
+     * @return position of the placeholder or -1 if not found
+     */
+    private int findLoadMoreCommentsPlaceholderPosition(String fullName, int positionHint) {
+        if (0 <= positionHint && positionHint < mVisibleComments.size()
+                && mVisibleComments.get(positionHint).getFullName().equals(fullName)
+                && mVisibleComments.get(positionHint).getPlaceholderType() == Comment.PLACEHOLDER_LOAD_MORE_COMMENTS) {
+            return positionHint;
+        }
+
+        for (int i = 0; i < mVisibleComments.size(); i++) {
+            Comment comment = mVisibleComments.get(i);
+            if (comment.getFullName().equals(fullName) && comment.getPlaceholderType() == Comment.PLACEHOLDER_LOAD_MORE_COMMENTS) {
+                return i;
+            }
+        }
+
+        return -1;
+    }
+
     private void expandChildren(ArrayList<Comment> comments, ArrayList<Comment> newList) {
         if (comments != null && comments.size() > 0) {
             for (Comment comment : comments) {