Reducing if/else statement complexity?

Question

I have a concise if/else statement below:

function () {
    if (elem.attr('data-src-1') === '' && elem.attr('data-src-2') === '') {
      // scenario a
    } else if (elem.attr('data-src-1') === '' && elem.attr('data-src-2') !== '') {
      // scenario b
    } else if (elem.attr('data-src-1') !== '' && elem.attr('data-src-2') === '') {
      // scenario c
    } else {
      // scenario d
    }
}

which is returning a complexity of 7 by strict linting rules. I need to reduce its complexity to 6 but can't see how to make it more concise?

You could just check data-src-1 (only) once in an if block, then in an inner if block, have scenarios a and b with the two states of data-src-2. Then in an else block, check data-src-2 and do scenarios c and d. — Katana314, Jun 14 '18 at 15:23
Cyclomatic complexity is a poor measure of code quality. I really have no idea why anyone uses it. — Liam, Jun 14 '18 at 15:28
So you'll end up re-writing this to be less readable to get it under 7...Craziness. — Liam, Jun 14 '18 at 15:30

score 2 · Answer 1 · answered Jun 14 '18 at 15:38

2

More readable one (at least for me)

let data1 = elem.attr('data-src-1') === ''
let data2 = elem.attr('data-src-2') === ''

if (data1)
    !data2 ? console.log(" scenario a ")  : console.log(" scenario b ") 
else
    data2 ?  console.log(" scenario c ") : console.log(" scenario d ")

answered Jun 14 '18 at 15:38

Margon

511
2
10

You could take it one step further and put it all into one giant ternary operation. `data1 ? (!data2 ? console.log(" scenario a ") : console.log(" scenario b ") ) : (data2 ? console.log(" scenario c ") : console.log(" scenario d "))`, but this looks horrible haha – Rich Jun 14 '18 at 15:42
But your logic is wrong, `data2` is a/b and `!data2` is c/d, not the way you have it. – Rich Jun 14 '18 at 15:45

Rich · Accepted Answer · 2018-06-14T15:40:09.950

1

This is more of a code review question, but you could combine the if 1='' into if/elses, then do the same for the interiors if/elses.

I think this is less readable, but it is technically less complex.

function() {
  if (elem.attr('data-src-1') === '') {
    if (elem.attr('data-src-2') === '') {
      // scenario a
    } 
    else {
      // scenario b
    }
  } else if (elem.attr('data-src-2') === '') {
    // scenario c
  } 
  else {
    // scenario d
  }
}

edited Jun 14 '18 at 15:40

answered Jun 14 '18 at 15:25

Rich

1,567
14
24

1

I re-structured my code to look more like this and it reduced its complexity to 6 - thanks! I will mark this as the answer when I can. – Rebecca O'Riordan Jun 14 '18 at 15:27

Reducing if/else statement complexity?

2 Answers2