-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restructure BotManager logic to use switch cases. #3
base: master
Are you sure you want to change the base?
Restructure BotManager logic to use switch cases. #3
Conversation
Wat betreft code ben ik het eens dat dit er beter uit ziet, maar ook ik heb nog geen test bot. @Sheepolution kun jij testen of dit werkt? |
The idea is good, but I'm not a fan of the nested switch statements. Don't forget to add |
} else if (interaction.customId.startsWith('coordinate_claim')) { | ||
ArtHandler.OnClaimPixel(messageInfo, interaction.customId.split('_')[2]); | ||
const messageInfo: IMessageInfo = await DiscordUtils.ParseInteractionToInfo(interaction); | ||
const id_parts = interaction.customId.split('_'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use idParts
instead.
const id_parts = interaction.customId.split('_'); | |
const idParts = interaction.customId.split('_'); |
const messageInfo: IMessageInfo = await DiscordUtils.ParseInteractionToInfo(interaction); | ||
const id_parts = interaction.customId.split('_'); | ||
switch(id_parts[0]) { | ||
case 'onboarding': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to place a break
after each case
.
const id_parts = interaction.customId.split('_'); | ||
switch(id_parts[0]) { | ||
case 'onboarding': { | ||
switch(id_parts[1]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a nested switch statement, create an InteractionHandler and place the inner switch statement in a method there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An InteractionHandler is a good idea! At that point, is there any reason to distinguish between Button, Modal, and ThreadMenu interactions? I feel like we can leave that to the BotManager, and let the InteractionHandler contain methods like onDiplomacyInteraction and onVoteInteraction to reduce the complexity.
This PR refactors the existing interaction handling logic to use switch statements instead of if-else blocks. This makes the code more concise, easier to read, and it better represents the Interaction categories.
Changes Made:
Disclaimer:
I have not tested this code as setting up a Discord Application would require a lot of work. I would greatly appreciate if someone with a test-application available could make sure the functionality is unchanged.