Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid IndexOutOfBoundsException at RecyclerView.getChildAdapterPosition #798

Merged
merged 2 commits into from
Feb 5, 2019
Merged

Avoid IndexOutOfBoundsException at RecyclerView.getChildAdapterPosition #798

merged 2 commits into from
Feb 5, 2019

Conversation

koji-1009
Copy link
Contributor

@koji-1009 koji-1009 commented Feb 5, 2019

Issue

Overview (Required)

  • Avoid IndexOutOfBoundsException when access RecyclerView.getChildAdapterPosition.

Links

Screenshot

Before After

@@ -58,8 +58,13 @@ open class TimetableCurrentTimeLineDecoration(
}

protected fun calcLineHeight(parent: RecyclerView, currentTime: Long): Float {
val originView = parent.getChildAt(0)
val originStartUnixMillis = groupAdapter.getItem(parent.getChildAdapterPosition(originView))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, Probably originView is null.
So you can fix.Can you fix it?
parent.getChildAt(0) ?: return 0F

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I did!

すいません!
対応したはずなのですが、別の対応をした方が良いということでしょうか?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent.getChildAt(0)でnullが返ってきているのでparent.getChildAdapterPosition(originView)が-1を返してきているのではという説があって、
val originView = parent.getChildAt(0) ?: return 0F
こうすれば解決するかもということです 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちら、修正前のコードになり、修正後のコードにはご指摘の対応を入れてある認識です……!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あ、すいません。
下のコードは不要ではないか、という指摘ですね。直します!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

既存のコードも同じ感じで直したいんですが、今日リリース予定で時間がとれないので😇

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正しました。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😇

@jmatsu-bot
Copy link
Collaborator

@koji-1009
Copy link
Contributor Author

Don't close #794

  • The issue is reproducible now
  • LeakCanary alerts now

===

#794 はクローズ対象としていません

  • 起票の事象は解決していないこと
  • LeakCanaryが警告を出していること

@jmatsu-bot
Copy link
Collaborator

Apk comparision results

Property Summary
New File Size 8459928 Bytes. (8.07 MB)
File Size Change -248 Bytes. (-0.24 KB)
Download Size Change -548 Bytes. (-0.54 KB)
New Method Reference Count 62705
Method Reference Count Change 0
New Number of dex file(s) 1
Number of dex file(s) Change 0

Generated by 🚫 Danger

@jmatsu-bot
Copy link
Collaborator

Asserted successfully. 💯

Generated by 🚫 Danger

Copy link
Member

@takahirom takahirom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing fast 🏎

@takahirom takahirom merged commit 2668b0b into DroidKaigi:master Feb 5, 2019
@koji-1009
Copy link
Contributor Author

Thanks!

@koji-1009 koji-1009 deleted the wakamiya/794_cate_recyclerview_no_position branch February 5, 2019 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants