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

Optional support for OptionHelper class. #36

Closed portlek closed 2 years ago

portlek commented 2 years ago

Issue Checklist

Affected Modules

Command

Description

Add java.util.Optional return type for each opt*** methods. For example;

    /**
     * Guarantees a String option value by providing a default value.
     *
     * @param event        The slash command event to get options from
     * @param option       The option we want
     * @param defaultValue The fallback option in case of the absence of the option value
     * @return The provided option, or the default value if the option is not present
     */
    @Nullable
    @Contract("_, _, !null -> !null")
    public static String optString(@NotNull SlashCommandEvent event, @NotNull String option, @Nullable String defaultValue) {
        List<OptionMapping> options = event.getOptionsByName(option);

        return options.isEmpty() ? defaultValue : options.get(0).getAsString();
    }

    /**
     * Guarantees a String option value by providing a default value.
     *
     * @param event        The slash command event to get options from
     * @param option       The option we want
     * @param defaultValue The fallback option in case of the absence of the option value
     * @return The provided option, or the default value if the option is not present
     */
    @NotNull
    public static Optional<String> optString(@NotNull SlashCommandEvent event, @NotNull String option, @Nullable String defaultValue) {
        return Optional.ofNullable(optString(event, option, defaultValue));
    }
Andre601 commented 2 years ago

Kinda pointless to have imo

portlek commented 2 years ago

it's a utility class so idk. but the purpose of this type of classes is this actually :d

Andre601 commented 2 years ago

A simple return OptionMapping == null ? defValue : OptionMapping#asWhatever() is sufficient enough. Creating an Optional just for this is a waste of time and resources. Also,there can never be more than one OptionMapping with a specific key.

portlek commented 2 years ago

if you think on this class, Optional.ofNullable(event.getOption("optionName")) is also enough for each opt**** methods. my point is this class called helper class which can have helper methods for every option mapping related methods.

Chew commented 2 years ago

To my knowledge, you can't have the same method name + same parameters with a different return type.

As has been stated, I don't really see the point of this. There's a whole debate of whether Optionals are good or not, I'm on the side of don't really care but optX already does everything I assume you'd want (you're guaranteed an option) which was the ultimate point of the thing.