masecla22 / Reddit4J

A library which aims to attain full coverage of the Reddit API
MIT License
48 stars 21 forks source link

Improve validation and error reporting in getSubreddit() #13

Closed espertus closed 1 year ago

espertus commented 1 year ago

Is your feature request related to a problem? Please describe. I am creating a (toy) Reddit client with Reddit4J I have encountered two problems with Reddit4J.getSubreddit(String name):

  1. If the subreddit does not exist, no error is reported. Instead, an instance of RedditSubreddit is returned where every member has the zero value.
  2. If the argument has invalid characters, such as ";", the application crashes with an org.jsoup.HttpStatusException.

Describe the solution you'd like

I propose (and am willing to implement):

  1. Adding JavaDoc to make the interface clearer (i.e., whose responsibility it is to make sure that the passed name is valid).
  2. Performing validation of the name and throwing IllegalArgumentException (or whatever you prefer) if the name is invalid.
  3. Returning null or throwing IllegalArgumentException (or whatever you prefer) if the subreddit does not exist.

Describe alternatives you've considered I could wrap the method in my own method to do all of the above.

Additional context These are the rules for subreddit names:

Names cannot have spaces (e.g., "r/bookclub" not "r/book club"), must be between 3-21 characters, and underscores ("_") are the only special characters allowed. Avoid using solely trademarked names (e.g., "r/FansOfAcme" not "r/Acme").

masecla22 commented 1 year ago

Yes, this is a good change.

Implementing validation on the client should lead to fewer resources being used as we're able to catch invalid requests faster. One thing I'll mention is to add a clear message inside the IllegalArgumentException.

I'm also not sure how well documented the Reddit API is, so I guess some further testing might be required to catch all edge cases.

masecla22 commented 1 year ago

Fixed in #14 and also in #15