How can I improve this 2000 lines function in vanilla Javascript?-CodePudding

I'm trying to build a salary tax calculator based on official numbers and rules in my country and I have this ugly monstrosity function, thousands of lines long. I'm curious to learn how I can improve this function.

I want to make it:

Shorter, less code
Easily extensible. Next year we will get new numbers

Numbers and cities in the code below are just examples. The actual code has about 500 if statements and 290 case rows per if statement:

export const getTax = (salary: number, city: string): number => {
  if (salary >= 1701 && salary <= 1800) {
    switch (city) {
      case 'TOKYO': return 126;
      case 'PARIS': return 136;
      case 'LONDON': return 145;
      case 'MADRID': return 193;
      // and many more case rows
      default: return 112;
    }
  }
  if (salary >= 1801 && salary <= 1900) {
    switch (city) {
      case 'TOKYO': return 135;
      case 'PARIS': return 186;
      case 'LONDON': return 176;
      case 'MADRID': return 192;
      // and many more case rows
      default: return 143;
    }
  }
  if (salary >= 1901 && salary <= 2000) {
    switch (city) {
      case 'TOKYO': return 212;
      case 'PARIS': return 209;
      case 'LONDON': return 188;
      case 'MADRID': return 211;
      // and many more case rows
      default: return 244;
    }
  }
  // and maybe 500 more if statements

CodePudding user response：

If there was a formula for calculating the tax for a given salary in a given city, I would just make an object with those formulas as the value for each city and simply call the appropriate formula based on the city. e.g.

taxes = {
  'LONDON' : (salary) => .1 * salary   1000,
  'TOKYO' : (salary) => .5 * salary   2455,
}

console.log(taxes['LONDON'](2500))
console.log(taxes['TOKYO'](2000))

Failing that, you could make an object of the salary and city tax values:

const tax = {
  1800 : { 'TOKYO' : 126, 'PARIS' : 136, 'LONDON' : 145, 'MADRID' : 193, 'default' : 112 },
  1900 : { 'TOKYO' : 135, 'PARIS' : 186, 'LONDON' : 176, 'MADRID' : 192, 'default' : 143 },
  2000 : { 'TOKYO' : 212, 'PARIS' : 209, 'LONDON' : 188, 'MADRID' : 211, 'default' : 244 }
}

const getTax = (salary, city) => {
  taxScale = tax[Object.keys(tax).find(k => k >= salary)]
  return taxScale?.[city] || taxScale.default
}

console.log(getTax(1830, 'LONDON'))
console.log(getTax(1966, 'MADRID'))
console.log(getTax(1348, 'NEW YORK'))

CodePudding user response：

Don't put the long data in the JavaScript. Put it in a CSV spreadsheet instead, and then parse the spreadsheet in order to construct the getTax function. For example, with a spreadsheet like

          1701-1800   1801-1900   1901-2000
TOKYO     126         135         212
PARIS     136         186         209
LONDON    145         176         188

You could use something like the node-csv package, and with the parse function, you'd get an array of arrays similar to

[
  ['', '1701-1800', '1801-1900', '1901-2000'],
  ['TOKYO', 126, 135, 212],
  ['PARIS', 136, 186, 209],
  ...
]

And then construct the conditions from that, something like:

const amountByCity = Object.fromEntries(
  data.slice(1).map(([city, ...amounts]) => [city, amounts])
);
const ranges = data[0].slice(1).map(str => str.split('-').map(Number));
const getTax = (salary, city) => {
  const rangeIndex = ranges.findIndex((lower, upper) => salary >= lower && salary <= upper);
  return amountByCity[city]?.[rangeIndex] ?? amountByCity['default'][rangeIndex];
};

CodePudding user response：

Somthing like that ?

cityVal = 
  { '1701-1800': { default: 112, TOKYO: 126, PARIS: 136, LONDON: 145, MADRID: 193 }
  , '1801-1900': { default: 143, TOKYO: 135, PARIS: 186, LONDON: 176, MADRID: 192 }
  , '1901-2000': { default: 244, TOKYO: 212, PARIS: 209, LONDON: 188, MADRID: 211 }
//...
  }

const getTax = (salary,  city ) => 
  {
  let range = Object.keys(cityVal).find( k =>
      {
      let [min,max] = k.split('-').map(Number)
      return (salary >= min && salary <= max)  
      })
  // if (!range) throw error no range on this salary...
  
  return cityVal[range][city] || cityVal[range].default
  }


// test 
console.log( getTax(1850,'LONDON')  )  // 176
console.log( getTax(1850,'xxx')  )     // 143