Being elegant in JavaScript

[ashfurrow]

Today my colleague provided a different approach to some code, here: https://github.com/artsy/peril-settings/pull/101/files#r253081362 To be fair, I'd just copy/pasted the code, so getting feedback on it was extra-easy.

Here's the code I had:

  const prs = prMerges
        .map(msg => msg.match(numberExtractor) && msg.match(numberExtractor)![1])
        .filter(pr => pr)
        .map((pr: any) => parseInt(pr))

And here's @jonallured's suggestion:

const prs = prMerges.map(message => {
  const matches = message.match(numberExtractor) || []
  const capture = matches[1]
  return parseInt(capture)
}).filter(Boolean)

Defaulting matches to [] works because [][1] returns undefined, and parseInt(undefined) returns NaN, and Boolean(NaN) returns false (for filter()). Very cool.

This wouldn't work in Swift for so many reasons, but that's okay. It's cool that each language has its own idioms.

[jonallured]

OK and now Jon will use this situation to describe something that frustrates him:

If a function returns an array, it should not also sometimes return null. It's so much easier to work with an empty array than a null. I believe that it was a mistake for the match function to be designed this way. Let's compare:

const regex = /asdf/
if (message.match(regex)) {
 // do a thing
} else {
  // do a different thing
}

This looks ok to me - we're throwing away the return of match so the fact that a miss on the regex returns falsy actually works to our advantage. Let's consider it the other way, if match returned an empty array when there was a miss:

const regex = /asdf/
if (message.match(regex).length === 0) {
 // do a thing
} else {
  // do a different thing
}

I still think that looks fine, but when we want to use the return of the match, then things look way better:

const regex = /asdf/
const matches = message.match(regex)
if (matches.length === 0) {
 // do a thing
  console.log(matches.length)
} else {
  // do a different thing
}

Now we have a local called matches and can be sure it's always an array and use it as such. Working with arrays is great because we can do all sorts of fun things like map, filter, forEach, etc. OTOH null is sorta boring - you can't do anything fun with it, you end up having to coerce it to something else.

Anyway, that's my Friday afternoon, unrequested rant, thanks for listening!

[ashfurrow]

@jonallured that makes sense! But not every language can be as simple as Ruby 😉

[jonallured]

Not with that attitude! 😝