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

Add programmatic view support #151

Merged
merged 1 commit into from
Mar 11, 2017
Merged

Add programmatic view support #151

merged 1 commit into from
Mar 11, 2017

Conversation

elihart
Copy link
Contributor

@elihart elihart commented Mar 9, 2017

@elihart
Copy link
Contributor Author

elihart commented Mar 9, 2017

I will follow up later with a processor change to create a view type when one isn't explicity defined

@elihart elihart force-pushed the eli/view_2.0 branch 2 times, most recently from e6f2abe to 5d99775 Compare March 9, 2017 23:55
* can only guarantee that view types are unique within a library. If you need to use {@link
* EpoxyModelWithView} models from different libraries or modules together you can define a view
* type manually to avoid this small collision chance. A good way to manually create value is by
* creating an R.id. value in an ids resource while, which will guarantee a unique value.
Copy link
Contributor

Choose a reason for hiding this comment

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

class ViewTypeManager {
  private final Map<Class<? extends EpoxyModel>, int> viewTypeMap = new HashMap<>();

  public int getViewType(EpoxyModel model) {
    Class<? extends EpoxyModel> modelClass = model.getClass();
    if (!viewTypeMap.contains(modelClass)) {
      int viewType = -viewTypeMap.size()-1;
      viewTypeMap.put(modelClass, viewType);
      return viewType;
    } else {
      return viewTypeMap.get(modelClass);
    }
  }
}

How about using a singleton to generate unique View types for every model class?

Copy link
Contributor Author

@elihart elihart Mar 10, 2017

Choose a reason for hiding this comment

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

@ngsilverman thanks for looking! yep, something like that would definitely work. The main issue is this can't work across multiple modules. They would instantly clash since they all start at -1 and go down. By using a hash we at least have a pretty high chance of not having a collision. The other thing I considered is that a hash of the class name is reproducible, whereas something like this could arbitrarily result in a different value if more model classes are added/deleted/implementation changes or whatever. it seems useful to have a fairly reproducible viewtype so if people do have a collision it is reproducible.

On the other hand, if we did it this way we could basically rule out library model sharing because they would clash. That might be safer than saying it probably won't be a hash collision. I think I can add debug checking for models with a view type collision being used in the same adapter at runtime though, and alert the user to use a manual id in that case, so that takes the risk away from random unknown collisions.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I should have clarified: My suggestion is to use the singleton at runtime, not during the annotation processing. Hence it should work across modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wow, brilliant! I think that could be perfect. I had too much tunnel vision on accomplishing this during processing, but your idea is much more simple and elegant

@elihart elihart merged commit 6497da3 into 2.0 Mar 11, 2017
@elihart elihart deleted the eli/view_2.0 branch March 11, 2017 01:25
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