segmentio / eslint-config

Segment's ESLint configurations.
9 stars 6 forks source link

Warn on cyclomatic complexity > 11 #48

Closed ndhoule closed 8 years ago

ndhoule commented 8 years ago

20 is ESLint's default. Haven't used the complexity rule much so we'll see how this pans out, I'm open to adjusting it up or down.

Set at a warning for now, I think that'll probably be best in the long term too.

Closes #8

ndhoule commented 8 years ago

cc @dominicbarnes @stephenmathieson

stephenmathieson commented 8 years ago

20 is way too high. you'd need a minimum of 40 different tests to fully cover a function with a cyclomatic complexity of 20. https://en.wikipedia.org/wiki/Cyclomatic_complexity#Implications_for_software_testing

stephenmathieson commented 8 years ago

this is how stupid a function with a cyclomatic complexity of 20 looks:

function foo(bar) {
  if (!bar) return 'hi nathan';

  if (bar == 1) {
    return 'something';
  }

  if (bar == 2) {
    return 'something';
  }

  if (bar == 3) {
    return 'something';
  }

  if (bar == 4) {
    return 'something';
  }

  if (bar == 5) {
    return 'something';
  }

  if (bar == 6) {
    return 'something';
  }

  if (bar == 7) {
    return 'something';
  }

  if (bar == 8) {
    return 'something';
  }

  if (bar == 9) {
    return 'something';
  }

  if (bar == 10) {
    return 'something';
  }

  if (bar == 11) {
    return 'something';
  }

  if (bar == 12) {
    return 'something';
  }

  if (bar == 13) {
    return 'something';
  }

  if (bar == 14) {
    return 'something';
  }

  if (bar == 15) {
    return 'something';
  }

  if (bar == 16) {
    return 'something';
  }

  if (bar == 17) {
    return 'something';
  }

  if (bar == 18) {
    return 'something';
  }

  return 'bananas';
}
dominicbarnes commented 8 years ago

I don't know why, but I thought the default was 10 (that's what I have set in my own eslint config)

ndhoule commented 8 years ago

Feel free to suggest alternative values.

dominicbarnes commented 8 years ago

I'm fine starting with 20, we can crank it up as time goes on.

fwiw, the usage of 10 has been mostly fine, but it does warn a little early imo.

ndhoule commented 8 years ago

I just read somewhere that 11 is an "industry standard." I'm going to pick that without actually validating the claim and if we hate it we can change it.