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

Comparers #929

Merged
merged 5 commits into from Jan 11, 2018
Merged

Comparers #929

merged 5 commits into from Jan 11, 2018

Conversation

Joe4evr
Copy link
Contributor

@Joe4evr Joe4evr commented Jan 8, 2018

As suggested in #912.

Also, if there are other suggestions for the containing class name, make them before this gets merged.

public static IEqualityComparer<IGuild> GuildComparer => _guildComparer ?? (_guildComparer = new EntityEqualityComparer<IGuild, ulong>());
public static IEqualityComparer<IChannel> ChannelComparer => _channelComparer ?? (_channelComparer = new EntityEqualityComparer<IChannel, ulong>());
public static IEqualityComparer<IRole> RoleComparer => _roleComparer ?? (_roleComparer = new EntityEqualityComparer<IRole, ulong>());

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMessage comparer as well?

Copy link
Contributor Author

@Joe4evr Joe4evr Jan 8, 2018

Choose a reason for hiding this comment

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

Good call.

EDIT: Wait, wasn't there something that it's not guaranteed message IDs will remain globally unique?

Decided to make a specialized implementation just to be safe.

if (xNull ^ yNull)
return false;

return x.Channel.Id.Equals(y.Channel.Id) && x.Id.Equals(y.Id);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we really need to check channel IDs as well as message IDs, snowflakes in messages should be unique globally.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we definitely don't. All snowflakes generated by Discord are guaranteed to be unique, unless they are explicitly duplicated (e.g. default general channel, everyone role)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vaguely recall a Discord dev saying that message IDs being globally unique can't be guaranteed indefinitely.

@foxbot foxbot merged commit b5e7548 into discord-net:dev Jan 11, 2018
@Joe4evr Joe4evr deleted the Comparers branch January 11, 2018 02:32
FiniteReality pushed a commit to FiniteReality/Discord.Net that referenced this pull request May 5, 2018
* Add entity equality comparers

* Fix namespace #whoops

* Add Message comparer.

* Add comment explaining the specialized implementation

* Remove specialized implementation, as per feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants