0

I have a function that return some informations about employees from an object database

I'm facing some complexity ESlint Problems, so i need to find a way to minimize it or find a smart way to do this instead of using a whole set of if statements.

I'm also trying to find a way to something like this: if I have name, I don't need to do the id verification and vice-versa and i just don't know how to do this... ‍♂️

The function must receive as argument an object of options that will determinate how it will behave:

  • name: the first name or last name of the person that need to be searched at database
  • id: the id of the person that need to be searched

e.g:

getEmployeesCoverage({ name: 'Sharonda' });
getEmployeesCoverage({ name: 'Spry' });
getEmployeesCoverage({ id: '4b40a139-d4dc-4f09-822d-ec25e819a5ad' });

I have 3 conditions:

  1. verify if i receive any arguments e.g name or id is undefined
  2. verify if name exists in database
  3. verify if id exists in database

I tried this:

function getEmployeesCoverage({ name, id }) {

  if (employees.find((employee) => employee.id === id)) {
    return getEmployeeById(id); // 3. verify if id exists in database
  }
  if (
    employees.find((employee) => employee.lastName === name) ||
    employees.find((employee) => employee.firstName === name)
  ) {
    return getEmployeeByName(name); // 2. verify if the name exists in database
  }
  if (!name || !id) { 
    return getAllEmployees(); // 1. verify if i have any args e.g name or id undefined
  } 
  throw new Error('Invalid Information');
}
Rafo
  • 511
  • 3
  • 17
  • Why are you already looking in the `employees` array to find by id or name, before then calling the respective function that appears to do the same thing again? Your function should only check whether the arguments are passed and then delegate to the respective find function. – Bergi Jan 13 '22 at 17:17
  • What's wrong with the complexity? Seems simple enough, only 4 cases. What rule settings are you using for eslint? – Bergi Jan 13 '22 at 17:18
  • I have this error ```Function 'getEmployeesCoverage' has a complexity of 6. Maximum allowed is 5.``` I couldnt understand how is calculated the complexity of a block or function. To me seems either that have n4 of complexity... [ESlint complexity](https://eslint.org/docs/rules/complexity) – Rafo Jan 13 '22 at 18:16
  • @Bergi i did this just because i wished do this verification upfront and leave the function do its only thing – Rafo Jan 13 '22 at 18:17
  • I suppose it counts the `||` as two possible control flow paths. – Bergi Jan 13 '22 at 20:12
  • And I would argue that your implementation is broken. It runs the `find` loops even if `id` (or `name` respectively) are `undefined`. And if you pass one of `id` or `name` but it's not a match, it will instead return all employees. I think cyclomatic complexity should be the least of your worries. First, properly define the interface of the function, or even better split it up and only really call the individual `get…` function that you want. – Bergi Jan 13 '22 at 20:16

3 Answers3

1

Cyclomatic complexity is just a style suggestion that ESLint provides to encourage you to use fewer code branches (e.g. ifs) in a single function. You can ignore the warning or disable it with ESLint pragmas (the rule in question is named complexity).

If you want a single function that is able to query in different ways (as you're doing in your code), it stands to reason that your function will need to branch out based on the input data.

For example, Python functions do this all the time by querying which kwargs were and weren't supplied to an "overloaded" function and the function changes behavior based on the suppied args. I'm not sure why your ESLint is configured with such a low complexity value, or maybe it comes with a low value.

Last time I used ESLint I found myself disabling three or four "code style" suggestions right away. I personally think it's overly opinionated by default.

Mike Clark
  • 10,027
  • 3
  • 40
  • 54
  • Thank you for your answer mike! unfortunately i can't disable this eslint rule because when i push it to github there's a github actions evaluator that runs eslint before check if the function works. – Rafo Jan 13 '22 at 18:05
  • @Rafo Who imposed this github action on your repository? You might want to ask the person who installed this rule for help. – Bergi Jan 13 '22 at 20:17
1

Perhaps you can do like;

var employee = o.id   ? employees.find(e => e.id === o.id) :
               o.name ? employees.find(e => e.lastName === o.name || e.firstName === o.name)
                      : undefined;

So if ESLint complains for nested ternary perhaps you may try;

var employee = (o.id   && employees.find(e => e.id === o.id)) ||
               (o.name && employees.find(e => e.lastName === o.name || e.firstName === o.name));

and if all fails you get false.

Redu
  • 25,060
  • 6
  • 56
  • 76
  • Thank you for your answer @Redu! I found your solution very clever, unfortunately this gives me another ESlint error [no-nested-ternary](https://eslint.org/docs/rules/no-nested-ternary) – Rafo Jan 13 '22 at 18:29
  • Then perhaps logical shortcircuits may help. – Redu Jan 13 '22 at 19:01
0

I resolve that problem by just checking the arguments in the main function:

function getEmployeesCoverage({ name, id } = {}) {
  if (name) return getEmployeeByName(name);
  if (id) return getEmployeeById(id);
  if (!name || !id) return getAllEmployees();
}

And then calling the unique job function that verifies if that argument exist in database and if not throw and error.

function getEmployeeByName(name) {
  if (employees.find((employee) => employee.lastName === name)
  || employees.find((employee) => employee.firstName === name)) {
    const {
      id: employeeId,
      firstName,
      lastName,
      responsibleFor,
    } = employees.find((employee) => employee.lastName === name)
    || employees.find((employee) => employee.firstName === name);
    return {
      id: employeeId,
      fullName: `${firstName} ${lastName}`,
      species: getSpeciesByIds(...responsibleFor).map((animal) => animal.name),
      locations: getSpeciesByIds(...responsibleFor).map((animal) => animal.location),
    };
  }
  throw new Error('Invalid Informations');
}
Rafo
  • 511
  • 3
  • 17