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

Improve Example documentation. #610

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class GetGuildExample {
public static void main(String[] args) {
/*
* Make sure you have a HypixelAPI object set up. You can see how this is done by going to
* the ExampleUtil class.
* the ExampleUtil class (ExampleUtil.java) .
firetrqck marked this conversation as resolved.
Show resolved Hide resolved
*
* See the finally{} block below for how to shutdown this API once you're all done.
*/
Expand All @@ -36,17 +36,17 @@ public static void main(String[] args) {
try {
/*
* We'll be fetching the guild's stats using its ID for this example, but guilds can
* also be looked up using their name, or one of their members' Minecraft UUIDs.
* also be looked up by their name, or one of their member's Minecraft UUIDs.

Choose a reason for hiding this comment

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

Original message is correct.

Copy link
Author

Choose a reason for hiding this comment

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

They have the same meaning but the edit is more grammatically correct and imo roles off the tongue easier.

Choose a reason for hiding this comment

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

The correct way to say it is, I believe, to drop the comma after "name" and use members' instead of member's

We'll be fetching the guild's stats by its ID for this example, but guilds can
also be looked up using their name or one of their members' Minecraft UUIDs.

Copy link
Contributor

Choose a reason for hiding this comment

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

"members" should definitely be plural, since the alternative would be "one of their member". As for "members'" vs "members's", I think it's just a matter of style, but the former is normally preferred (Firefox's spellcheck highlights the latter, if that counts for anything).

Copy link
Author

Choose a reason for hiding this comment

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

fixed

* - HypixelAPI.getGuildByName(String)
* - HypixelAPI.getGuildByPlayer(UUID)
*/
String guildId = ExampleUtil.GUILD_ID;

/*
* Here, we store the response from the API in our variable.
* Here, we store the response from the API.
*
* We call `.get()` at the end so that we can use the reply in the same thread.
* The downside is that the current thread freezes (or "blocks") until the API responds.
* The downside is that this is synchronous operation.

Choose a reason for hiding this comment

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

Original message should be kept as we are here losing the fact that the performances are dependant of the API response time.

Copy link
Author

Choose a reason for hiding this comment

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

The current thread being blocked until something happening (in this case the API responding) is the definition of synchronous operation(which dependent the time the API takes to respond is how long the thread takes to be unblocked hence the performance being dependent on api response time)

Copy link
Contributor

Choose a reason for hiding this comment

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

The Hypixel API is very accessible, so it often attracts a lot of new developers, which I catered these examples toward. From a newbie's perspective, I worry that "synchronous" might send them down a rabbit hole that makes their learning curve feel a lot steeper. Words like "blocks" and "thread" are there so that those who understand those concepts get the gist.

* If this is a problem for you, instead use:
*
* .whenComplete((apiReply, error) -> {
Expand All @@ -61,7 +61,7 @@ public static void main(String[] args) {
System.err.println("Oh no, our API request failed!");

/*
* If an ExecutionException is thrown, it's typically because of an API error.
* If an ExecutionException is an arbitary error, typically because of an API error.
firetrqck marked this conversation as resolved.
Show resolved Hide resolved
* Use `getCause()` to determine what the actual problem is.
*/
e.getCause().printStackTrace();
Expand Down Expand Up @@ -127,8 +127,8 @@ public static void main(String[] args) {
/*
* Finally, we'll print some information about each member in the guild.
*
* This might print out A LOT, so you may want to comment the following line out if you're
* focusing on some of the guild's other info.
* This might print out upto 125 seperate usernames, so you may want to comment the following line out if you're
* focusing on other info.

Choose a reason for hiding this comment

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

This 125 members limitation is only an in-game limitation, the endpoint could technically returns more members if there was more players in the guild.

Copy link
Author

@firetrqck firetrqck Aug 8, 2023

Choose a reason for hiding this comment

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

is this better in reflected with cefffd2?

*/
printGuildMembers(guild.getMembers());
}
Expand Down