3
\$\begingroup\$

I'm making a Python-like range() function (see Python docs) using JavaScript. This function can take from 1 to 3 arguments. There are 3 variables (start, stop and step) that need to have some value, either from arguments, or a default value.

  • If only one argument is passed, start is 0, stop is the first argument and step is 1.
  • If two arguments are passed, start is the first argument, stop is the second argument and step is 1.
  • If three arguments are passed, start is the first argument, stop is the second argument and step is the third argument.

I'm aware that since ECMAScript 6 JavaScript has default parameters, but the situation here is more complex. Therefore, I used rest parameters to get an array of all arguments and then assign them to appropriate variables.

const range = (...args) => {
  if (args.length < 1 || args.length > 3) {
    throw new Error('This function takes between 1 and 3 arguments')
  }
  const [start, stop, step] = {
    1: [0, ...args, 1]
   ,2: [...args, 1]
   ,3: args
  }[args.length]
}

I'm using the solution presented in the article Replacing switch statements with Object literals. The object keys are numbers of arguments and values are arrays with values for start, stop and step. I select the array which matches args.length and use destructuring assignment to assign the array elements respectively to start, stop and step.

This code is quite short, but I'm not sure whether it's readable. I came up with an alternative solution, using simple if-else blocks. I have to declare the variables at the beginning here, because let and const are block-scoped, so declaring them inside the if-else blocks would have no effect on the rest of the function.

const range = (...args) => {
  let [start, stop, step]
  if (args.length === 1) {
    ;[start, stop, step] = [0, ...args, 1]
  } else if (args.length === 2) {
    ;[start, stop, step] = [...args, 1]
  } else if (args.length === 3) {
    ;[start, stop, step] = args
  } else {
    throw new Error('This function takes between 1 and 3 arguments')
  }
}

This version maybe is more readable, but I don't like it very much. It repeats the [start, stop, step] fragment four times, and also I can't use const (and I'm trying to use const whenever possible).

Another solution that came to my mind is to assume as default the situation when two or three parameters are passed, and when one parameters is passed, assign start to stop and 0 to start.

const range = function(start, stop, step = 1) {
  if (arguments.length < 1 || arguments.length > 3) {
    throw new Error('This function takes between 1 and 3 arguments')
  }
  if (stop === undefined) {
    ;[start, stop] = [0, start]
  }
}

This also doesn't look good to me. I think that for someone reading this code it would be very difficult to understand what it does. Also, this code forces me to use a regular function instead of arrow function (because arrow functions don't have arguments) and violates two ESLint rules which I'm using: prefer-rest-params and no-param-reassign.

What is the best way to solve this problem? Should I modify one of the solutions that I already have, or should I try something completely different?

Edit

I made a variation of the third option which doesn't violate any of my ESLint rules:

const range = (...args) => {
  if (args.length < 1 || args.length > 3) {
    throw new Error('This function takes between 1 and 3 arguments')
  }
  let [start, stop, step = 1] = args
  if (stop === undefined) {
    ;[start, stop] = [0, start]
  }
}
\$\endgroup\$
  • \$\begingroup\$ Actually, aside from handling invalid number of arguments, it's possible to do this in one line: const [start, stop, step] = [[0, ...args, 1], [...args, 1], args][args.length - 1] but that's extremely unreadable. \$\endgroup\$ – Michał Perłakowski Oct 27 '16 at 21:39
  • 1
    \$\begingroup\$ Saying of the 3rd example that "I think that for someone reading this code it would be very difficult to understand what it does" is absurd. It's quite clear imo, and probably the best of the three. The first example is the most "difficult," but still clear enough to anyone familiar with es6, and rather clever. The if else version is the worst to my taste, but all are fine. Why do you begin lines with ;? \$\endgroup\$ – Jonah Oct 28 '16 at 5:35
  • \$\begingroup\$ @Jonah I'm using npm's coding style. \$\endgroup\$ – Michał Perłakowski Oct 28 '16 at 13:52
  • 1
    \$\begingroup\$ I like the edited version. I'd replace the if statement with ;[start, stop] = stop === undefined ? [0, start] : [start, stop]. fair enough on the npm style guide, though fwiw i think it's pretty nasty. i prefer guidelines like these: github.com/airbnb/javascript. but, style of that sort isn't something i care too much about.... \$\endgroup\$ – Jonah Oct 28 '16 at 16:41
  • \$\begingroup\$ Related: codereview.stackexchange.com/questions/167537/… \$\endgroup\$ – Patrick Roberts Jul 7 '17 at 13:46
2
\$\begingroup\$

I like the edited version. I'd suggest just one small change:

const range = (...args) => {
  if (args.length < 1 || args.length > 3) {
    throw new Error('This function takes between 1 and 3 arguments')
  }
  let [start, stop, step = 1] = args
  ;[start, stop] = (stop === undefined) ? [0, start] : [start, stop]
}

Aside from reducing the visual clutter of the if statement, in general I find that expressions (which always execute) are easier to reason about than branching statements, even though the resulting logic is the same.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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