Skip to content

Commit

Permalink
Fix Extension coupling to precise model class
Browse files Browse the repository at this point in the history
Summary:
Since Extension (and Binder itself) can accept an Interface or abstract Class as the definition of the model, it's not correct to strictly prohibit exact type changes.

For example, if we want to bind a Drawable as a background, we should not limit the model to bind one exact Drawable type, but rather allow any Drawable.

And with the help of generics on Extension and Binder types we can be sure that `shouldUpdate` and `bind`/`unbind` methods will be called with the same model type for both current and new extension/binder.

Also, arguably, check for `binder` type is not needed too, since we use it (`newExtension.binder.getClass()`) as a key to retrieve the `currentExtension`, thus we'll have the same `binder` type guaranteed.

Reviewed By: pasqualeanatriello

Differential Revision: D23539921

fbshipit-source-id: a9016a2dbed89c5fc5e8d5dff2d99442c97ad63d
  • Loading branch information
colriot authored and facebook-github-bot committed Sep 4, 2020
1 parent 6f07790 commit f547036
Showing 1 changed file with 1 addition and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ private static <MOUNT_CONTENT> void addExtension(
// list.
boolean found = false;
for (int i = extensions.size() - 1; i >= 0; i--) {
if (extensions.get(i).equalTypes(extension)) {
if (extensions.get(i).binder.getClass() == extension.binder.getClass()) {
extensions.remove(i);
found = true;
break;
Expand Down Expand Up @@ -419,23 +419,10 @@ public static <MODEL, CONTENT> Extension<MODEL, CONTENT> extension(
return new Extension<>(model, binder);
}

boolean equalTypes(Extension extension) {
return model.getClass() == extension.model.getClass()
&& binder.getClass() == extension.binder.getClass();
}

boolean shouldUpdate(
final Extension<MODEL, CONTENT> prevExtension,
final @Nullable Object currentLayoutData,
final @Nullable Object nextLayoutData) {
if (!equalTypes(prevExtension)) {
throw new IllegalArgumentException(
"The types to operate on do not match!\nNew model: "
+ model
+ ", old model: "
+ prevExtension.model);
}

return binder.shouldUpdate(prevExtension.model, model, currentLayoutData, nextLayoutData);
}

Expand Down

0 comments on commit f547036

Please sign in to comment.