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

Adds User#getMutualGuilds() #252

Merged
merged 7 commits into from
Feb 8, 2017
Merged

Adds User#getMutualGuilds() #252

merged 7 commits into from
Feb 8, 2017

Conversation

ArsenArsen
Copy link
Contributor

Adds a utility method User#getMutualGuilds() which returns a collection of guilds that are common for the User and its JDA instance.

@@ -243,4 +255,4 @@ public String toString()
return text;
}
}
}
}
Copy link
Collaborator

@Almighty-Alpaca Almighty-Alpaca Feb 7, 2017

Choose a reason for hiding this comment

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

pls (re)add a new line char at the end of the file

@@ -125,6 +129,14 @@ protected void handleResponse(Response response, Request request)
}

@Override
public Collection<Guild> getMutualGuilds() {
return getJDA().getGuilds()
.parallelStream()
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if we should really use a parallel stream here, they can be very slow

@@ -208,6 +210,24 @@ public boolean equals(Object o)
User getUserById(String id);

/**
* Gets all {@link Guild}s that contain all given users as their members.
Copy link
Member

Choose a reason for hiding this comment

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

This has to be a full qualified path to the Guild object like {@link net.dv8tion.jda.core.entities.Guild Guild}
Always include the path and an alias name.

@@ -208,6 +210,24 @@ public boolean equals(Object o)
User getUserById(String id);

/**
* Gets all {@link Guild}s that contain all given users as their members.
*
* @param users The users which all the returned {@link Guild}s must contain.
Copy link
Member

Choose a reason for hiding this comment

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

The description of a parameter needs to be aligned in the next line as you can see in the javadoc template.
Same for docs below.

*
* @param users The users which all the returned {@link Guild}s must contain.
*
* @return Unmodifiable list of all {@link Guild} instances which have all {@link User}s in them.
Copy link
Member

Choose a reason for hiding this comment

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

Missing @throws with full qualified name and newline description
Include the plural s in the alias name for the entity that you are linking Users

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.*;
Copy link
Member

Choose a reason for hiding this comment

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

This wasn't needed. We only wildcard for 5+ imports from the same package usually.

Copy link
Member

Choose a reason for hiding this comment

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

Wildcard came from the use of Arrays which set the value to 5, thus it wildcarded.

Args.notNull(users, "users");
for(User u : users)
{
if(u == null)
Copy link
Member

Choose a reason for hiding this comment

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

This should use Args.notNull, too.

@@ -125,6 +128,12 @@ protected void handleResponse(Response response, Request request)
}

@Override
public List<Guild> getMutualGuilds()
Copy link
Member

Choose a reason for hiding this comment

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

There should also be an option to add other users here, imo

Copy link
Member

Choose a reason for hiding this comment

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

(This was countered in chat. Decision was not to include it here as it would be redundant to JDA#getMutalGuilds(User...)

@@ -111,6 +113,14 @@
RestAction<PrivateChannel> openPrivateChannel();

/**
* Finds and collects all {@link net.dv8tion.jda.core.entities.Guild Guild} instances that contain this {@link net.dv8tion.jda.core.entities.User User} within the current {@link net.dv8tion.jda.core.JDA JDA} instance.<br>
Copy link
Member

Choose a reason for hiding this comment

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

Technically, <br> should be on the next line In this case however, there is no line that is part of this paragraph after this line so there actually shouldn't even be a <br> as the following <p> handles the paragraph separation.

@@ -111,6 +113,14 @@
RestAction<PrivateChannel> openPrivateChannel();

/**
* Finds and collects all {@link net.dv8tion.jda.core.entities.Guild Guild} instances that contain this {@link net.dv8tion.jda.core.entities.User User} within the current {@link net.dv8tion.jda.core.JDA JDA} instance.<br>
* <p>This method is a shortcut for {@link net.dv8tion.jda.core.JDA#getMutualGuilds(User...) JDA.getMutualGuilds(User)}.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Paragraph doesn't need to be closed for future reference.
The start of a new paragraph should be separate from the previous paragraph by an extra line of space in the javadoc comment.

@DV8FromTheWorld DV8FromTheWorld merged commit 9fd11c8 into discord-jda:development Feb 8, 2017
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.

4 participants