Discord.js role checking optimization

Question

I'm currently building a discord verification bot, one of the features of this bot if to check if you have 1 role from each required category and then produce of summary of your roles at the end. Currently I have a working solution that functions exactly how I want it too but it uses a large stack of if's, I'm quite new to js but have been told to stay away from using large if/if else if stacks where possible. I've looked into switch cases but cant really see how I would apply those here, so I'm wondering if there's a more optimized way to achieve what I need. My current code is as follows:

var hasrole = false;
var role = "";
var subrole = "";
var istype1 = false;
var istype2 = false;
var istype3 = false;
var istype4 = false;
var counter = 0;

if (message.member.roles.cache.find(r => r.id === "736032207248031765")) {
    istype1 = true;
    role = "Type 1";
    hasrole = true;
    counter ++;
}
if (message.member.roles.cache.find(r => r.id === "736032822053175306")) {
    istype2 = true;
    role = "Type 2";
    hasrole = true;
    counter ++;
}
if (message.member.roles.cache.find(r => r.id === "736032670013980702")) {
    role = "Type 3";
    hasrole = true;
    counter++
    if (message.member.roles.cache.find(r => r.id === "736032716411371581")) {
        istype1 = true;
        subrole = "Type 1";
    } else if (message.member.roles.cache.find(r => r.id === "736032780122849340") {
        istype2 = true;
        subrole = "Type 2";
    } else {
        istype3 = true;
    }
}
if (message.member.roles.cache.find(r => r.id === "736032870703038534")) {
    istype4 = true;
    role = "Type 4";
    hasrole = true;
    counter ++;
}
if (message.member.roles.cache.find(r => r.id === "736033471474172025")) {
    istype2 = true;
    role = "Type 5";
    hasrole = true;
    counter ++;
}
if (counter >= 1) {
    message.channel.send("More than 1 main role, please have only 1");
    return;
} else if (hasrole == false) {
    message.channel.send("No main role please get one")
    return;
}

ApocalypseCalculator ApocalypseCalculator · Accepted Answer · 2020-11-19T17:22:12

Ok so I'm assuming your code works fine and just needs a little optimization.

My #1 suggestion is that you use arrays and iterate over your types.

My #2 suggestion is to not hardcode your role ids.

My #3 suggestion is to maybe read through your code and try to understand it because it looks quite messy right now.

Your code in your type 3 role check is confusing and your type 5 role check validates the role 2 value.

Here is my version of your code

const roles = ['736032207248031765', '736032822053175306', '736032670013980702', '736032870703038534', '736033471474172025'];
let userRoles = [];

roles.forEach(r => {
    if(message.member.roles.cache.has(r)){
        userRoles.push(r);
    }
});

if (userRoles.length >= 1) {
    message.channel.send("More than 1 main role, please have only 1");
    return;
} else if (userRoles.length == 0) {
    message.channel.send("No main role please get one")
    return;
}

As you can see this is much more convenient than a whole chain of ifs.

Discord.js role checking optimization

1 Answers