How can i simplify the following If code?

Question

I have the following code in JS (using last ES2020), the current code check if the type is 'asporto' or 'takeaway' and adding to current date hours from a setup fo type 'takeaway' or 'asporto' and if another setup called limiteOreTakeaway/Asporto is !== '00:00:00' I have to check if the current time is greater than the limiteOreTakeaway/Asporto, so I'm splitting the string limiteOreTakeaway/Asporto to Hours and Minutes and making my checks if the current date is > that limiteOreTakeaway/Asporto I'm adding to curDate the hours from setup + 24hours.

My code looks like this:

const curDate = new Date();
    if (this.carrello.tipo === 'asporto') {
      if (this.negozioSetup.limiteOreAsporto !== '00:00:00') {
        // controllo se l'orario limite è impostato (orariolimite != 00:00)
        const ore = parseInt(
          this.negozioSetup.limiteOreAsporto.split(':')[0],
          10
        );
        const minuti = parseInt(
          this.negozioSetup.limiteOreAsporto.split(':')[1],
          10
        );
        // controllo se l'ora di adesso ha superato l'orario limite
        if (curDate.getHours() > ore) {
          curDate.setHours(
            curDate.getHours() + this.negozioSetup.oreMinimeAsporto + 24
          );
         // se l'ora corrente è uguale controllo se i minuti limite hanno superato il limite
        }else if (curDate.getHours() === ore && curDate.getMinutes() > minuti) {
            // se l'orario corrente è maggiore di quello limite aggiungo 24 ore alla data corrente
            curDate.setHours(
              curDate.getHours() + this.negozioSetup.oreMinimeAsporto + 24
            );
        }
      } else {
        // orario limite disabilitato proseguo con l'aggiunta delle ore minime tra gli ordini
        curDate.setHours(
          curDate.getHours() + this.negozioSetup.oreMinimeAsporto
        );
      }
    } else if (this.carrello.tipo === 'takeaway') {
      if (this.negozioSetup.limiteOreTakeaway !== '00:00:00') {
        const ore = parseInt(
          this.negozioSetup.limiteOreTakeaway.split(':')[0],
          10
        );
        const minuti = parseInt(
          this.negozioSetup.limiteOreTakeaway.split(':')[1],
          10
        );
        if (curDate.getHours() > ore) {
          curDate.setHours(
            curDate.getHours() + this.negozioSetup.oreMinimeTakeaway + 24
          );
        }else if (curDate.getHours() === ore && curDate.getMinutes() > minuti) {
          curDate.setHours(
            curDate.getHours() + this.negozioSetup.oreMinimeTakeaway + 24
          );
        }
      } else {
        curDate.setHours(
          curDate.getHours() + this.negozioSetup.oreMinimeTakeaway
        );
      }
    }

I would like to simplify it. If it's possible, optimize it by using the last ES2020 available tools.

Answer 1

Use functions instead of repeating yourself The only difference between the tipo === 'asporto' branch and the tipo === 'takeaway' branch is the property name that gets passed into setHours: either .oreMinimeAsporto or .oreMinimeTakeaway. Create a function that takes that value as a parameter, then call it with either .oreMinimeAsporto or .oreMinimeTakeaway instead. That will about halve the size of your code.

const curDate = new Date();
const { tipo } = this.carrello;
if (tipo === 'asporto' || tipo === 'takeaway') {
    const casedProp = tipo[0].toUpperCase() + tipo.slice(1);
    changeDate(
        currDate,
        this.negozioSetup['oreMinime' + casedProp],
        this.negozioSetup['limiteOre' + casedProp],
    );
}

Split only once and destructure to grab the ore and minti values at once. You can also use Number instead of parseInt(str, 10), and use .map to apply the transformation to both elements at once:

const [ore, minuti] = this.negozioSetup.limiteOreAsporto.split(':').map(Number);

Call functions once rather than multiple times when possible - you don't need to call getHours every time, better to call it once and store the result in a variable.

const currHours = curDate.getHours();
const currHoursWithOffset = currHours + oreMinime;

Return early to avoid indentation hell - reading lots of {} blocks can make code more confusing to read than it needs to be. In the case of 00:00:00, consider running the one line that's needed and then returning, rather than having a large if/else.

The branches of currHours > ore and curDate.getHours() === ore && curDate.getMinutes() > minuti also contain the same logic inside the branches, so combine them into one if statement:

const changeDate = (date, oreMinime, limiteOre) => {
    const currHours = curDate.getHours();
    const currHoursWithOffset = currHours + oreMinime;
    if (limiteOre === '00:00:00') {
        curDate.setHours(currHoursWithOffset);
        return;
    }
    const [ore, minuti] = limiteOre.split(':').map(Number);
    if (currHours > ore || currHours === ore && curDate.getMinutes() > minuti) {
        curDate.setHours(
            currHours + oreMinime + 24
        );
    }
};
const curDate = new Date();
const { tipo } = this.carrello;
if (tipo === 'asporto' || tipo === 'takeaway') {
    const casedProp = tipo.slice(1).toUpperCase() + tipo.slice(1);
    changeDate(currDate, this.negozioSetup['oreMinime' + casedProp]);
}

You could put the values into variables first if you think the combined statement is difficult to read (I'll leave that to you, I don't understand the language) - eg

const cond1 = currHours > ore;
const cond2 = currHours === ore && curDate.getMinutes() > minuti;
if (cond1 || cond2) {
  // ...
}

Alternative to multiple separate properties On a broader scale, having to hard-code the correlation between this.carrello.tipo and the this.negozioSetup.oreMinime- property is a bit odd. If possible, consider rearranging your data so that the two oreMinime and limiteOre values are in an object instead of being two separate properties. Then you just need to use bracket notation to look up the value on the object:

ore: {
  asporto: { minime: <value>, takeaway: <value> },
  takeaway: { minime: <value>, takeaway: <value> },
}

Then you can:

changeDate(currDate, this.negozioSetup.ore[tipo]);

and change changeDate's parameters as needed.