Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/src/main/java/com/togetherjava/tjplays/Bot.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ private static JDA createBot(Config config) {
private static List<SlashCommand> getCommands(Config config) {
return List.of(
new PingCommand(),
new Game2048Command()
new Game2048Command(),
new JavaQuizCommand(config.openAIApiKey())
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.togetherjava.tjplays.games.game2048;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this class part of the games.game2048 package?


import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.List;

public record QuizQuestion(
@JsonProperty("question") String question,
@JsonProperty("choices") List<String> choices,
@JsonProperty("correct") int correctIndex
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NIT: It took me a little bit to understand what correctIndex represents. Something like correctAnswerIndex would help.

) {
@JsonCreator
public QuizQuestion {}

public String getQuestion() {
return question;
}

public List<String> getChoices() {
return choices;
}

public int getCorrectIndex() {
return correctIndex;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.togetherjava.tjplays.games.game2048;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this class part of the games.game2048 package?


import com.togetherjava.tjplays.services.chatgpt.ChatGptService;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.util.Optional;

public class TriviaManager {
private final ChatGptService gpt;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please properly name this variable to chatGptService.

private final ObjectMapper mapper = new ObjectMapper();

public TriviaManager(String openAiKey) {
this.gpt = new ChatGptService(openAiKey);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should give people the opportunity to pass ChatGptService down from those who instantiate TriviaManager. The way the code currently is, for every TriviaManager instance there is a ChatGptService instance which is redundant and all TriviaManager instances can share the same ChatGptService.

}

public Optional<QuizQuestion> fetchRandomQuestion() {
String prompt = "Provide one Java trivia question as a JSON object like"
+ " {\"question\":\"...\",\"choices\":[\"A\",\"B\",\"C\",\"D\"],\"correct\":<index>}";
Optional<String> raw = gpt.ask(prompt, "java quiz");
if (raw.isEmpty()) {
return Optional.empty();
}
try {
QuizQuestion question = mapper.readValue(raw.get(), QuizQuestion.class);
return Optional.of(question);
} catch (Exception e) {
return Optional.empty();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package com.togetherjava.tjplays.listeners.commands;

import com.togetherjava.tjplays.games.game2048.QuizQuestion;
import com.togetherjava.tjplays.games.game2048.TriviaManager;

import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent;
import net.dv8tion.jda.api.events.interaction.component.ButtonInteractionEvent;
import net.dv8tion.jda.api.interactions.commands.build.Commands;
import net.dv8tion.jda.api.interactions.components.buttons.Button;

import java.util.List;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

public final class JavaQuizCommand extends SlashCommand {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once again, a JavaDoc would be useful here for future contributors as this is public facing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs to extend SlashCommandAdapter not SlashCommand

private static final String COMMAND_NAME = "javaquiz";

private final TriviaManager triviaManager;
private final ConcurrentHashMap<String, QuizQuestion> activeQuestions = new ConcurrentHashMap<>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why ConcurrentHashMap and not just HashMap?


public JavaQuizCommand(String openAiKey) {
super(Commands.slash(COMMAND_NAME, "Get a random Java trivia question"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once changing the extend, this will need to become:

super(COMMAND_NAME, "Get a random Java trivia question", CommandVisibility.GUILD);

this.triviaManager = new TriviaManager(openAiKey);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just like in this code review comment, I believe that we should let callers pass down an instance of TriviaManager.

}

@Override
public void onSlashCommand(SlashCommandInteractionEvent event) {
event.deferReply().queue();

Optional<QuizQuestion> question = triviaManager.fetchRandomQuestion();
if (question.isEmpty()) {
event.getHook().editOriginal("Could not fetch a quiz question. Try again later.").queue();
return;
}

QuizQuestion q = question.get();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's use proper variable names instead of a single letter for readability. q could be quizQuestion and the current question can be quizQuestionOptional.

List<String> choices = q.getChoices();

StringBuilder sb = new StringBuilder();
sb.append("**Java Quiz**\n\n");
sb.append(q.getQuestion()).append("\n\n");
for (int i = 0; i < choices.size(); i++) {
sb.append("`").append(i + 1).append(")` ").append(choices.get(i)).append("\n");
}
Copy link
Copy Markdown
Member

@christolis christolis Mar 10, 2026

Choose a reason for hiding this comment

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

Let's not use sb as a variable name. This whole highlighted code block is also unreadable.

A lot of things could be changed here, and the removal of StringBuilder is a good place to start from.

Firstly, we should use String.format(...) as it's more readable and does not have .append() being repeated a lot.

Secondly, sending a MessageEmbed would allow for the code to be cleaner but also for the UI/UX to be significantly more structured.


String messageId = event.getHook().editOriginal(sb.toString())
.setActionRow(
Button.primary(COMMAND_NAME + "-1-" + event.getUser().getId(), "1"),
Button.primary(COMMAND_NAME + "-2-" + event.getUser().getId(), "2"),
Button.primary(COMMAND_NAME + "-3-" + event.getUser().getId(), "3"),
Button.primary(COMMAND_NAME + "-4-" + event.getUser().getId(), "4")
)
.complete()
.getId();

activeQuestions.put(messageId, q);
}

@Override
public void onButtonInteraction(ButtonInteractionEvent event) {
String buttonId = event.getButton().getId();
if (buttonId == null || !buttonId.startsWith(COMMAND_NAME)) {
return;
}

if (!buttonId.contains(event.getUser().getId())) {
event.reply("This isn't your quiz!").setEphemeral(true).queue();
return;
}

QuizQuestion q = activeQuestions.remove(event.getMessageId());
Comment thread
christolis marked this conversation as resolved.
Outdated
if (q == null) {
event.reply("This quiz has already been answered.").setEphemeral(true).queue();
return;
}

int chosen = Character.getNumericValue(buttonId.charAt(COMMAND_NAME.length() + 1)) - 1;
boolean correct = chosen == q.getCorrectIndex();

String result = correct
? "Correct! The answer is: **" + q.getChoices().get(q.getCorrectIndex()) + "**"
: "Wrong! The correct answer was: **" + q.getChoices().get(q.getCorrectIndex()) + "**";
Comment thread
christolis marked this conversation as resolved.
Outdated

String userId = event.getUser().getId();

event.editMessage(event.getMessage().getContentRaw() + "\n\n" + result)
.setActionRow(
Button.primary(COMMAND_NAME + "-1-" + userId, "1").asDisabled(),
Button.primary(COMMAND_NAME + "-2-" + userId, "2").asDisabled(),
Button.primary(COMMAND_NAME + "-3-" + userId, "3").asDisabled(),
Button.primary(COMMAND_NAME + "-4-" + userId, "4").asDisabled()
)
.queue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import java.time.Duration;
import java.util.List;
import java.util.Objects;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps you need to rebase since this import does not exist on the main branch?

import java.util.Optional;

/**
Expand Down
1 change: 1 addition & 0 deletions gradle.properties
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revert this file please - it's not needed to be changed.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
org.gradle.java.home=C:/Users/dudem/.gradle/jdks/eclipse_adoptium-21-amd64-windows/jdk-21.0.9+10
Comment thread
EJ-Edwards marked this conversation as resolved.
Outdated
Loading