Chew / JDA-Chewtils

Chew's fork of JDA-Applications/JDA-Utilities, with support for modern features such as Slash Commands, Context Menus, and more.
https://chew.pro/JDA-Chewtils
Apache License 2.0
74 stars 23 forks source link

Improve OptionHelper methods #38

Closed Andre601 closed 2 years ago

Andre601 commented 2 years ago

Pull Request

Pull Request Checklist

Please follow the following steps before opening this PR.
PRs that do not complete the checklist will be subject to denial for missing information.

Pull Request Information

Check and fill in the blanks for all that apply:

Description

Improves the methods within the OptionHelper class as follows:

Andre601 commented 2 years ago

So... I now added optX(SlashCommandEvent, String) options which return optX(SlashCommandEvent, String, <defValue>) with some specific default values.

For anything nullable is this simply null, for optBoolean is it false and for the numbers (long, double) is it 0 and 0.0 respectively.

I didn't apply any @Contract annotations to the nullables, simply because they seem to not be needed (IntelliJ complains about improper usage of (@Contract("_, _") since it requires a arg -> result or smth...)

If I should change any of the default values, lmk.

Chew commented 2 years ago

I'm half debating having event.getUser() (and whatever else) for the default values if they are in the event. Not sure about others but I'd find falling back to the invoking user more useful than null. Same with MessageChannel I think?

Andre601 commented 2 years ago

I'm half debating having event.getUser() (and whatever else) for the default values if they are in the event. Not sure about others but I'd find falling back to the invoking user more useful than null. Same with MessageChannel I think?

Personally, I see it like that null should be returned as it simplifies if checks. With a nullable return value can I simply check if the option is null and do things while with the other I have to compare it with the event user or smth else which I don't think is that nice.

Like, if I want to return an error when the user was not given would I need to call SlashCommandEvent#getUser() just to check if a default value was returned, while with null a simple x == null check can be performed...

Returning null here would signify that the option was either not filled out (in case of an optional... option) or the key was wrong... Either way, for me makes it much more sense to have null returned.

Chew commented 2 years ago

Yeah I suppose that's easier, and more consistent.