Value in array set in a lambda expression stays null-CodePudding

I'm using JDA (Java Discord API), a Java wrapper for Discord's public API. I'm trying to use a lambda expression to store my Discord bot's message ID in an array, then use the message ID outside of the lambda to add reactions to the bot's message. However, the value in the array stays null. Code:

String[] botMessageId = new String[1]; // Supposed to store the message ID of the bot's response to a command
event.getChannel().sendMessage("This is a message").queue(message -> {
        botMessageId[0] = message.getId(); // Bot sends a message to the channel and the array stores the message ID
});
event.getChannel().addReactionById(botMessageId[0], "\u1F1E6"); // Supposed to add a reaction to the message that the bot just sent
event.getChannel().addReactionById(botMessageId[0], "\u1F1E7");
event.getChannel().addReactionById(botMessageId[0], "\u1F1E8");

Error:

java.lang.IllegalArgumentException: Message ID may not be null
    at net.dv8tion.jda.internal.utils.Checks.notNull(Checks.java:69)
    at net.dv8tion.jda.internal.utils.Checks.isSnowflake(Checks.java:39)
    at net.dv8tion.jda.api.entities.MessageChannel.addReactionById(MessageChannel.java:1714)
    at net.dv8tion.jda.internal.entities.TextChannelImpl.addReactionById(TextChannelImpl.java:482)
    at com.beta.cipherer.commands.Commands.onGuildMessageReceived(Commands.java:47)
    at net.dv8tion.jda.api.hooks.ListenerAdapter.onEvent(ListenerAdapter.java:466)
    at net.dv8tion.jda.api.hooks.InterfacedEventManager.handle(InterfacedEventManager.java:96)
    at net.dv8tion.jda.internal.hooks.EventManagerProxy.handleInternally(EventManagerProxy.java:88)
    at net.dv8tion.jda.internal.hooks.EventManagerProxy.handle(EventManagerProxy.java:70)
    at net.dv8tion.jda.internal.JDAImpl.handleEvent(JDAImpl.java:160)
    at net.dv8tion.jda.internal.handle.MessageCreateHandler.handleInternally(MessageCreateHandler.java:97)
    at net.dv8tion.jda.internal.handle.SocketHandler.handle(SocketHandler.java:36)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onDispatch(WebSocketClient.java:952)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onEvent(WebSocketClient.java:839)
    at net.dv8tion.jda.internal.requests.WebSocketClient.handleEvent(WebSocketClient.java:817)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onBinaryMessage(WebSocketClient.java:990)
    at com.neovisionaries.ws.client.ListenerManager.callOnBinaryMessage(ListenerManager.java:385)
    at com.neovisionaries.ws.client.ReadingThread.callOnBinaryMessage(ReadingThread.java:276)
    at com.neovisionaries.ws.client.ReadingThread.handleBinaryFrame(ReadingThread.java:996)
    at com.neovisionaries.ws.client.ReadingThread.handleFrame(ReadingThread.java:755)
    at com.neovisionaries.ws.client.ReadingThread.main(ReadingThread.java:108)
    at com.neovisionaries.ws.client.ReadingThread.runMain(ReadingThread.java:64)
    at com.neovisionaries.ws.client.WebSocketThread.run(WebSocketThread.java:45)

When putting the methods for adding reactions into the lambda, I get this error:

java.lang.IllegalArgumentException: ID may not be null
    at net.dv8tion.jda.internal.utils.Checks.notNull(Checks.java:69)
    at net.dv8tion.jda.internal.utils.Checks.notEmpty(Checks.java:75)
    at net.dv8tion.jda.api.utils.MiscUtil.parseSnowflake(MiscUtil.java:115)
    at net.dv8tion.jda.api.entities.MessageHistory.getMessageById(MessageHistory.java:322)
    at com.beta.cipherer.commands.StepCommands.onGuildMessageReceived(StepCommands.java:51)
    at net.dv8tion.jda.api.hooks.ListenerAdapter.onEvent(ListenerAdapter.java:466)
    at net.dv8tion.jda.api.hooks.InterfacedEventManager.handle(InterfacedEventManager.java:96)
    at net.dv8tion.jda.internal.hooks.EventManagerProxy.handleInternally(EventManagerProxy.java:88)
    at net.dv8tion.jda.internal.hooks.EventManagerProxy.handle(EventManagerProxy.java:70)
    at net.dv8tion.jda.internal.JDAImpl.handleEvent(JDAImpl.java:160)
    at net.dv8tion.jda.internal.handle.MessageCreateHandler.handleInternally(MessageCreateHandler.java:97)
    at net.dv8tion.jda.internal.handle.SocketHandler.handle(SocketHandler.java:36)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onDispatch(WebSocketClient.java:952)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onEvent(WebSocketClient.java:839)
    at net.dv8tion.jda.internal.requests.WebSocketClient.handleEvent(WebSocketClient.java:817)
    at net.dv8tion.jda.internal.requests.WebSocketClient.onBinaryMessage(WebSocketClient.java:990)
    at com.neovisionaries.ws.client.ListenerManager.callOnBinaryMessage(ListenerManager.java:385)
    at com.neovisionaries.ws.client.ReadingThread.callOnBinaryMessage(ReadingThread.java:276)
    at com.neovisionaries.ws.client.ReadingThread.handleBinaryFrame(ReadingThread.java:996)
    at com.neovisionaries.ws.client.ReadingThread.handleFrame(ReadingThread.java:755)
    at com.neovisionaries.ws.client.ReadingThread.main(ReadingThread.java:108)
    at com.neovisionaries.ws.client.ReadingThread.runMain(ReadingThread.java:64)
    at com.neovisionaries.ws.client.WebSocketThread.run(WebSocketThread.java:45)

Please let me know if there is a better way to achieve the result I need and why the value in the array is null.

CodePudding user response：

You can directly add the reactions in the queue callback:

event.getChannel().sendMessage("This is a message").queue(message -> {
        message.addReaction("U 1F1E6").queue();
        message.addReaction("U 1F1E7").queue();
        message.addReaction("U 1F1E8").queue();
});

The Unicode escapes you are attempting to use, such as \u1F1E8, do not work in java. You need to use UTF-16 escape codes, which would be "\uD83C\uDDE8" instead. However, JDA also supports the codepoint notations, such as "U XXXXX", which will automatically be converted for you by addReaction. In my example I used addReaction("U 1F1E8"), which converts the codepoint notation into the Unicode characters automatically.

Your code with the message array does not work, because queue callbacks are asynchronous. This means they run at a later time, which means you need to put your code into the callback, rather than below it.

CodePudding user response：

You're trying to dodge callback hell. It doesn't work.

There is a reason you have to do the whole .queue(foo -> ...) thing, and it's not 'just to annoy you'. Imagine it worked you like wanted (that the code runs immediately). Then obviously the API would just return that message ID.

You'd write this:

String botMessageId = 
event.getChannel().sendMessage("This is a message");

Much simpler. The reason it doesn't work that way is because that sendMessage thing is asynchronous. It won't run right away. You're just asking the system: Hey, uh, when you get around to it, go send this thing. On your own time, I'll just keep going while you take care of that.

The .queue() thing is an addendum: "... oh, and when you did finally get around to taking care of sending this thing, please do this immediately afterwards".

So, the code flow is:

You ask the system to send a message, and then run the foo -> x code afterwards.
The system is still a long way off from doing that.
You immediately keep going and call addReactionById. You're passing a null pointer here, as the botMessageId[0] = ... code hasn't run yet.
That code crashes.
Sometime later the system sends the message and runs your botMessageId[0] = ... code. Yelling into the void, as nobody will ever look at this. The only code that references botMessageId[0] has run and gone.

The solution is to move all the stuff you do with the botMessageId value into the queue call itself:

event.getChannel().sendMessage("This is a message").queue(message -> {
  event.getChannel().addReactionById(message.getId(), "\u1F1E6");
  event.getChannel().addReactionById(message.getId(), "\u1F1E7");
  event.getChannel().addReactionById(message.getId(), "\u1F1E8");
});

Of course, if as part of responding to a message you'd want to send another message, then you'd have a .queue(msg -> { ... }) inside another .queue(msg -> { ... }), so if this async model is used a lot, you end up with an endless cavalcade of ->, having to wrap all code that is of any interest into a lambda and passing it somewhere. Any attempt to debug this will be borderline impossible and the code is an unreadable mess. This is called 'callback hell'.

The correct solution in java land is to stop using async. It is highly unlikely you need async solutions. Mdoern CPUs can easily juggle thousands of threads. You can set their stack sizes quite low.

Of course, if this is API you did not write and have to use, that's not an alternative you can use here, unless the API also offers synchronous alternatives.

NB: The new String[1] trick is usually better written as AtomicRefererence<String> instead (slightly more idiomatic, AR has a few nice properties, and unlike arrays, has more sensible toString() and the like). You can use it as a hack to get around issues when the call is lambda based (with -> notation), but synchronous. For example, someList.stream().forEach(foo -> bar) is synchronous: The lambda will run once for each item in the list on the spot, and then the forEach method itself returns, vs. what you have, where the .sendMessage method returns almost immediately, but the actual sending the message and running your lambda occurs much later, in another thread most likely. So, when synchronous lambdas are happening you can use this trick and it will work. But if you're designing the API and this comes up, first consider rewriting it so lambdas aren't neccessary. You don't as a rule want to foist the burden of dealing with lack of exception/localvar/control-flow transparency on the callers of your API, after all.