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

No equality operators for IUser (and possibly others) #912

Closed
Foxtrek64 opened this issue Dec 23, 2017 · 10 comments
Closed

No equality operators for IUser (and possibly others) #912

Foxtrek64 opened this issue Dec 23, 2017 · 10 comments

Comments

@Foxtrek64
Copy link

Foxtrek64 commented Dec 23, 2017

This makes it impossible to use LINQ and other built-in methods for ensuring that objects are equal to each other. Will submit a pull request with this.

IReadOnlyCollection<IUser> usersReacted = await msg.GetReactionUsersAsync(emote.Name); // Returns list of 25 users (subset of everyone in guild)
IReadOnlyCollection<IUser> usersInGuild = sguser.Guild.Users; // Returns list of 50 users
IEnumerable<IUser> usersToRemove = usersInGuild.Except(usersReacted); // Should return everyone in the guild except those who reacted

usersToRemove ends up being the same as usersInGuild. I'll try and find a less-painful solution than doing things manually.

@khionu
Copy link
Contributor

khionu commented Dec 23, 2017

  1. You can use the IDs, instead. That's how I've done my Linq in bots, and I imagine how most people do.
  2. This is almost certainly how it would be done internally. If you wanted to PR it, you'd be free to (the acceptance of it to the lib would be up to the maintainers, though)

@Foxtrek64
Copy link
Author

I can wrangle the return type for GetReactionUsersAsync to be a RestUser, but the only common type between RestUser and SocketGuildUser (The return type of sguser.Guild.Users) is IUser where equality is broken. Thus, the only kind of thing I can think of doing is writing my own method that steps through both collections and only takes the unique values based on ID, but it would be far easier to just rely on the built-in methods.

@Joe4evr
Copy link
Contributor

Joe4evr commented Dec 23, 2017

Thus, the only kind of thing I can think of doing is writing my own method that steps through both collections and only takes the unique values based on ID

.Except() (and just about every other LINQ method) has an overload that takes in an IEqualityComparer<T> at which point you can just do something like this. No need to write your own methods.

@khionu
Copy link
Contributor

khionu commented Dec 23, 2017

Like I noted, you can always use the IDs

if (restUserObject.Id == socketUserObject.Id)
    return;

@FiniteReality
Copy link
Member

Keep in mind that we already tried something like this in #181 and it ended up being postponed almost indefinitely. However, this is a good place to discuss it properly, so cc @RogueException @foxbot

@Foxtrek64
Copy link
Author

Foxtrek64 commented Dec 23, 2017

There seem to be two solutions for this. Here's the code I'm using as suggested by @khionu:

IEnumerable<IUser> usersReacted = await msg.GetReactionUsersAsync(emote.Name);
IEnumerable<IUser> usersToRemove = (channel.Guild.Users).Where(x => usersReacted.All(y => x.Id != y.Id));

And the other option is implementing an IEqualityComparer<T> as suggested by @Joe4evr.

The solution I'm using now definitely works, though I would very much be in favor of seeing a handful of IEqualityComparers added in as a more long-term and maintainable solution.

@Joe4evr
Copy link
Contributor

Joe4evr commented Dec 23, 2017

If we're gonna add comparers to the base library, then I nominate to consider the one I linked, because it works perfectly and I've spent a good amount of time refining it to get lazy-initialization with low overhead.

@foxbot
Copy link
Member

foxbot commented Jan 8, 2018

@Joe4evr if you want to PR yours, seems good to me - I don't see any reason why we shouldn't have these

@Still34
Copy link
Member

Still34 commented Jan 8, 2018

I second IEqualityComparer.

@Joe4evr Joe4evr mentioned this issue Jan 8, 2018
@Still34
Copy link
Member

Still34 commented Apr 8, 2018

Should probably be closed now that comparers have been added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants