CODEWITHSUNDEEP
javascriptnode.jsdiscord.js
MH Discord
MH Discord

Reputation: 53

Remove if statements

I'm making a bot for Discord. To load the command when a user types it, I need to verify if the command exists, but now I have three times almost the same if statements. Is there a way to get everything in only one expression ?

    if (!client.commands[cat]) return;

    if (client.commands[cat][command]) {

        if (client.commands[cat][command].conf) {
            if (client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation) return message.reply(`you don't have the required elevation to use this command.`);
            if (client.commands[cat][command].conf.accountRequired)
                if (!(await client.accountsManager.findById(message.author.id))) return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
            return client.commands[cat][command].run(client, args, message);

        } else if (!args[0]) {
            if (client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation) return message.reply(`you don't have the required elevation to use this command.`);
            if (client.commands[cat][command].conf.accountRequired)
                if (!(await client.accountsManager.findById(message.author.id))) return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
            return client.commands[cat][command].run(client, args, message);

        } else if (!client.commands[cat][args[0]]) {
            if (client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation) return message.reply(`you don't have the required elevation to use this command.`);
            if (client.commands[cat][command].conf.accountRequired)
                if (!(await client.accountsManager.findById(message.author.id))) return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
            return client.commands[cat][command].run(client, args, message);

        } else { ...

Upvotes: 0

Views: 88

Answers (2)

VLAZ
VLAZ

Reputation: 29007

Let me represent more abstractly what the code looks like:

if (mainCondition1) {
  if (subCondition1) return result1;
  if (subCondition2)
    if (subCondition3) return result2;
   return result3;
} else if (mainCondition2) {  
  if (subCondition1) return result1;
  if (subCondition2)
    if (subCondition3) return result2;
   return result3;
} else if (mainCondition3) {
  if (subCondition1) return result1;
  if (subCondition2)
    if (subCondition3) return result2;
   return result3;
}

This makes it easier to reason about the code. It's also easier to see where exactly the repetition is and it's 

if (subCondition1) return result1;
if (subCondition2)
  if (subCondition3) return result2;
return result3;

Which is present three times under different conditions.

First of all

if (A) { 
  if (B) { 
   doSomething() 
  } 
}

behaves the same (and can be simplified to):

if (A && B) { 
 doSomething() 
}

since the code inside will only execute if both condition A and condition B are true. This is the first step, the repeated section can be shortened to:

if (subCondition1) return result1;
if (subCondition2 && subCondition3) return result2;
return result3;

Next is the construct:

if (A) {
  doSomething()
} else if (B) {
  doSomething()
}

can also be represented as simple boolean algebra:

if (A || B) {
  doSomething()
}

because the inner code will be executed if at least one of the conditions is correct.

Putting this rule into place, the three repeated sections (which are under three different conditions) collapse into a single block:

if (mainCondition1 || mainCondition2 || mainCondition3) {
  if (subCondition1) return result1;
  if (subCondition2 && subCondition3) return result2;
  return result3;
}

Now by just extracting the logic, you can also simplify the full expression:

//extract into variables - no evaluation overhead
const mainCondition1 = client.commands[cat][command].conf;
const mainCondition2 = !args[0];
const mainCondition3 = !client.commands[cat][args[0]];

//extract into functions - only evaluate if needed
const subCondition1 = () => client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation;
const subCondition2 = () => client.commands[cat][command].conf.accountRequired;
const subCondition3 = async () => !(await client.accountsManager.findById(message.author.id));

if (mainCondition1 || mainCondition2 || mainCondition3) {
  if (subCondition1()) 
    return message.reply(`you don't have the required elevation to use this command.`);
  if (subCondition2() && await subCondition3()) 
    return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
  return client.commands[cat][command].run(client, args, message);
}

Feel free to attach more meaningful names to the variables and functions. They should really reflect what the content would be and would make your code self-documenting. Since I don't actually know what the intent behind each is, here are my guesses to serve as an example of what this could look like:

const hasConfiguration = client.commands[cat][command].conf;
const noArgumentSupplied = !args[0];
const commandDoesNotSupportArgument = !client.commands[cat][args[0]];

const authorHasInsufficientElevation = () => client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation;
const accountRequiredForCommand = () => client.commands[cat][command].conf.accountRequired;
const accountDoesNotExist = async () => !(await client.accountsManager.findById(message.author.id));

if (hasConfiguration || noArgumentSupplied || commandDoesNotSupportArgument ) {
  if (authorHasInsufficientElevation()) 
    return message.reply(`you don't have the required elevation to use this command.`);
  if (accountRequiredForCommand() && await accountDoesNotExist()) 
    return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
  return client.commands[cat][command].run(client, args, message);
}

Upvotes: 2

KolCrooks
KolCrooks

Reputation: 534

Just use the ||(or) operator

    if (!client.commands[cat]) return;

    if (client.commands[cat][command]) {

        if (client.commands[cat][command].conf || !args[0] || !client.commands[cat][args[0]]) {
            if (client.elevationManager.calculateGuildElevation(message.author.id, message.guild) < client.commands[cat][command].conf.elevation) return message.reply(`you don't have the required elevation to use this command.`);
            if (client.commands[cat][command].conf.accountRequired)
                if (!(await client.accountsManager.findById(message.author.id))) return message.reply(`you must have an account to use this command ! Type \`,,account create\` to get one.`);
            return client.commands[cat][command].run(client, args, message);

        } else { ...

@VLAZ has a much better answer to this that shows better programming practices, Go look at his solution.

Upvotes: 0

Related Questions