JavaScript Coding Style – Is ‘Break’ a Code Smell?

code-qualitycoding-stylejavascript

I'm asking in terms of a loop, obviously break is important in switch statements. Whether or not switch statements themselves are code smells is a separate issue.

So consider the following use cases for iterating a data structure:

  1. You want to do something to the entire structure (no break needed)
  2. You want to do something to part of a data structure.
  3. You want to find something(s) in the data structure (which may or may not involve iterating the entire structure)

The above list seems more-or-less exhaustive to me, maybe I'm missing something there.

Case 1 can be thrown right out, we can use map/forEach. Case 2 sounds like filter or reduce would work. For case 3, needing to iterate the data structure to find something seems plain wrong, either the data structure itself should provide a relevant method or you are likely using the wrong data structure.

While not every javascript data structure (or implementation) has those methods it's trivially simple to write the relevant functions for pretty much any javascript data structure.

I saw this when researching but it is explicitly in the context of C/C++. I could understand how it would be more-or-less a necessity in C, but my understanding is that C++ has lambdas and many data structures are generally objects (e.g. std::vector, std::map), so I'm not sure I understand why the answers are all so universally in favor of break there, but I don't feel I know C++ well enough to even begin to comment.

I also realize that for some corner-case exceedingly large data structure the cost of iterating the entire structure may be unacceptably high, but I doubt those are very common when working even in node.js. Certainly it's the kind of thing you'd want to profile.

So I just don't see a use-case for break in today's javascript. Am I missing something here?

Best Answer

Having a break out of a loop is no different than having that loop get refactored out to a function of its own and a return statement in a guard clause.

while(condition) {
  if(test) { break; }
  doStuff;
}

vs

doMuchStuff();

function doMuchStuff() {
  while(condition) {
    if(test) { return; }
    doStuff;
  }
}

Those are effectively the same.

Single keywords are not code smells. They are tools for flow control. Many of them are variations on goto wrapped with some other level of safety to avoid the nightmare of spaghetti code.

In judging if a particular bit of code is problematic, it is necessary to look at how it works and if it is appropriate for the language constructs. The key word alone is not sufficient to establish a code smell. It might be a whiff in that it can be misused, but going on witch hunts because one sees a switch or break or some other flow control construct is counterproductive and leads to style guides that prevent people from writing understandable, straight forward code.

Related Topic