5

I was running our code through JSHint, and decided to switch checks against cyclomatic complexity, and then went on long refactoring sprint. One place though baffled me, here is a code snippet:

var raf = null //raf stands for requestAnimationFrame

if (window.requestAnimationFrame) {
  raf = window.requestAnimationFrame;
} else if (window.webkitRequestAnimationFrame) {
  raf = window.webkitRequestAnimationFrame;
} else if (window.mozRequestAnimationFrame) {
  raf = window.mozRequestAnimationFrame;
} else if (window.msRequestAnimationFrame) {
  raf = window.msRequestAnimationFrame;
} else {
  raf = polyfillRequestAnimationFrame;
}

Not surprisingly, in this implementation CM is 5, first attempt was to use solution from MDN:

var requestAnimationFrame = window.requestAnimationFrame || window.mozRequestAnimationFrame ||
                          window.webkitRequestAnimationFrame || window.msRequestAnimationFrame;
window.requestAnimationFrame = requestAnimationFrame;

Which simply looks like a hack to me (yes I know, majority of full time javascript programmers won't agree with me, however this is a prevalent opinion within our team). Doodling around my code, I found another hacks I could employ to full code-linter, among ones I was proud for about 5 second was usage of array comprehensions:

var rafs = [
             window.requestAnimationFrame,
             window.webkitRequestAnimationFrame,
             window.mozRequestAnimationFrame,
             window.msRequestAnimationFrame,
             polyfillRequestAnimationFrame
           ].filter(function (rafClosure) {
             return rafClosure !== null && rafClosure !== undefined;
           });
return rafs[0];

However I'm curios whether there is more or less standard practice of refactoring long branching code (that's not trivial to reduce)?

1
  • 1
    I wrote a couple relevant answers a while ago here and here regarding simplifying control flow, these techniques are basic and general but can reduce cyclomatic complexity in many cases. Apr 4, 2013 at 19:16

1 Answer 1

7

The first sample is just wrong. Not wrong in terms of the result, but the style is terrible. It is repetitive and error prone. The second sample is actually the step to do to obtain the readable, well written code. It's not a hack. It's just badly written code rewritten correctly. The fact that it reduces cyclomatic complexity doesn't surprise me, since there are less operations (at least in source; I'm pretty sure every decent compiler/interpreter will rewrite the first sample into the second one in all cases).

The third sample is more complicated to read for beginners who don't have enough practice with arrays, with filtering, with closures, etc. Personally, given the second sample, I would avoid the third one. Still, reduction in cyclomatic complexity is not surprising neither: you are migrating the complexity from your code to the filter method.

3
  • 3
    @nirth: Regardless of your team's opinions, you are programming in Javascript, and you should follow standard Javascript idioms even if they seem strange at first. Otherwise you will just incur pain (i.e. waste time and money) for no benefit. Apr 4, 2013 at 20:13
  • @kelvin cline You rise a good point, but we are not javascript programmers, and I try to keep code readable for us, and people who will be likely hired in future (who most probably won't be javascript programmers). Ammount of work we do in js is miniscule Apr 4, 2013 at 22:39
  • @MainMa You've convinced me I guess. Apr 5, 2013 at 9:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.