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

[suggest]Remove isSameAs instead of Item.id #682

Merged
merged 5 commits into from Feb 2, 2020

Conversation

@koji-1009
Copy link
Contributor

koji-1009 commented Feb 1, 2020

Issue

  • close N/A

Overview (Required)

com.xwray.groupie.Item.isSameAs is this code.

    /**
     * Whether two item objects represent the same underlying data when compared using DiffUtil,
     * even if there has been a change in that data.
     * <p>
     * The default implementation compares both view type and id.
     */
    public boolean isSameAs(Item other) {
        if (getLayout() != other.getLayout()) {
            return false;
        }
        return getId() == other.getId();
    }

getLayout() != other.getLayout() result is same as other is HogeItem result.
So I delete that methods.

And, getId() is following.

    private static AtomicLong ID_COUNTER = new AtomicLong(0);
    private final long id;

    public Item() {
        this(ID_COUNTER.decrementAndGet());
    }

    protected Item(long id) {
        this.id = id;
    }

    /**
     * If you don't specify an id, this id is an auto-generated unique negative integer for each Item (the less
     * likely to conflict with your model IDs.)
     * <p>
     * You may prefer to override it with the ID of a model object, for example the primary key of
     * an object from a database that it represents.
     *
     * @return A unique id
     */
    public long getId() {
        return id;
    }

Therefore, I add unique id.
(If we don't use a same layout, we don't request the uniqueness.)

Links

Screenshot

@koji-1009

This comment has been minimized.

Copy link
Contributor Author

koji-1009 commented Feb 1, 2020

see #684

@takahirom

This comment has been minimized.

Copy link
Member

takahirom commented Feb 1, 2020

Can you merge from master for fxing CI?

@koji-1009

This comment has been minimized.

Copy link
Contributor Author

koji-1009 commented Feb 1, 2020

Thanks!
Now merge it!

@koji-1009

This comment has been minimized.

Copy link
Contributor Author

koji-1009 commented Feb 1, 2020

Sorry, I was overlooked DividerItem class.

@jmatsu-bot

This comment has been minimized.

Copy link
Collaborator

jmatsu-bot commented Feb 1, 2020

Your apk has been deployed to https://deploygate.com/distributions/a43c35cae8b7ad6293fe93c3f3444bc34ddb5e24. Anyone can try your changes via the link.

Generated by 🚫 Danger

@jmatsu-bot

This comment has been minimized.

Copy link
Collaborator

jmatsu-bot commented Feb 1, 2020

No issue was reported. Cool!

Generated by 🚫 Danger

Copy link
Member

takahirom left a comment

Thanks! I don't have a strong opinion on which way to write.

@takahirom takahirom merged commit b38b90c into DroidKaigi:master Feb 2, 2020
3 checks passed
3 checks passed
ci/circleci: assemble_apk Your tests passed on CircleCI!
Details
ci/circleci: test_android Your tests passed on CircleCI!
Details
every_pull_request Workflow: every_pull_request
Details
@koji-1009 koji-1009 deleted the koji-1009:remove_issameas_instead_of_id branch Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.