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

Allows a datasource to return different chatItem instances for same message id #36

Merged
merged 1 commit into from
Feb 8, 2016

Conversation

diegosanchezr
Copy link
Contributor

Previously, due to the mapping between chatItem instance and presenter, a new presenter would be created. This fixes:

  1. That new presenter will have invalid itemVisibility property (would cause a crash when long pressing a text cell)
  2. If a new chatItem instance is returned by the dataSource for a previous existing message, but the previous chatItem is leaked, then the presenter would leak too (and it might try to update its cell, as in downloading an image)

This introduces ChatItemCompanion, a structure which attaches a ChatItem with its presenter and decoration attributes. This solves 1) and 2)

Still, a new presenter will be created if a different instance is returned by the datasource for a same message id. This will cause its internal state to be lost, so it's recommended not to keep any state in the presenter, or use an external storage to be able to recover it. We might revisit this in the future by reusing same presenters for same uids or adding API to copy state from one presenter to a new one.

@codecov-io
Copy link

Current coverage is 61.85%

Merging #36 into dev will increase coverage by +0.56% as of f15b1ac

@@              dev     #36   diff @@
=====================================
  Files          55      57     +2
  Stmts        2855    2894    +39
  Branches        0       0       
  Methods         0       0       
=====================================
+ Hit          1750    1790    +40
  Partial         0       0       
+ Missed       1105    1104     -1

Review entire Coverage Diff as of f15b1ac

Powered by Codecov. Updated on successful CI builds.

…dates.

Previously, due to the mapping between chatItem instance and presenter, a new presenter would be created. This fixes:
1) That new presenter will have invalid itemVisibility property (would cause a crash when long pressing a text cell)
2) If a new chatItem instance is returned by the dataSource for a previous existing message, but the previous chatItem is leaked, then the presenter would leak too (and it might try to update its cell, as in downloading an image)

This introduces ChatItemCompanion, a structure which attaches a ChatItem with its presenter and decoration attributes. This solves 1) and 2). Still new instances of presenters will be created for new instances of chatItems --> Presenters should avoid keeping any state in its internal storage.
diegosanchezr added a commit that referenced this pull request Feb 8, 2016
Allows a datasource to return different chatItem instances for same message id
@diegosanchezr diegosanchezr merged commit e7e58ae into badoo:dev Feb 8, 2016
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.

None yet

2 participants