What's the best practice for checking a variable in Javascript before iterating?

Question

Suppose I have a function like:

    let statement = 'Some text with [square brackets] inside [several] times'
    let tags = []
    tags = statement.match(/[^[\]]+(?=])/g)

    if (tags && tags.length > 0) {
        for (let t = 0; t < tags.length; t++) {
            tags[t] = tags[t].toLowerCase()
            tags[t] = tags[t].replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_')
        }
    }

    console.log(tags)

As you can see I'm implementing this check: tags && tags.length to avoid an error being thrown in case the string didn't have any elements in the square brackets and if the tags is empty.

Is there a more elegant way to do that?

Something is telling me it's not the best-case scenario to check the length of a variable to know whether it exists or not...

Thanks!

Answer 1

let tags = []
tags = statement.match(/[^[\]]+(?=])/g)

This is pointless. You're creating an array and then immediately discarding it.

Just do:

let tags = statement.match(/[^[\]]+(?=])/g)

---

Then, match will return either null or an array.

So you need to test for that. Since null isn't truthy and any array is, you can simply:

if (tags)

---

There is no need to test the length of the array at all.

If an array has a length of zero then the for loop you have will, itself be a suitable test. t < tags.length; will fail, and it will never enter the body of the loop.

Answer 2

You don't need to check for neither actually. You explicitly define tags and if it's length is 0 the for loop will never run anyway. What I would do is make sure that tags was always defined in this way:

let statement = 'Some text with [square brackets] inside [several] times';
let tags = statement.match(/[^[\]]+(?=])/g) || [];

for (let t = 0; t < tags.length; t++) {
    tags[t] = tags[t].toLowerCase();
    tags[t] = tags[t].replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_');
}

console.log(tags);

Answer 3

All answers so far are using a traditional imperative style but perhaps what you want is to move to functional style:

let statement = 'Some text with [square brackets] inside [several] times'
let tags = statement.match(/[^[\]]+(?=])/g)
              ?.map(t => t.toLowerCase())
              .map(t => t.replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_'))
              ?? []
console.log(tags)

The ? operator makes sure map only runs when not null or undefinied. If nothing matches the ? will return undefinied, short circuiting the map functions.

The map transforms the list into something else, so to lower case and then replace symbols with underscore.

The ?? transformsundefinied into something else. Note that ? will return undefinied when nothing matches. When nothing matches I want an empty array instead of undefinied, just because it makes more sense.

---

You may wonder what's the point of using functional style. In this particular case it's more readable and less error prone.

Functional style is declarative. Instead of saying what the computer must do, you say what it should do. For example, you don't have to know how map is implemented, only that it transform a list into another list. It doesn't matter if it's implemented with a while loop, a for loop, or even if it's splitting the array and running each part in parallel; the result of map is always the same, given the same input. This gives opportunity for optimization in future engine updates (e.g., execute map in parallel) and results in shorter, less error prone code. In imperative style not only there's no opportunity for any optimization, but the developer may also have to know unimportant details, like how an array index starts (e.g., zero or one) or what state must be changed in each loop iteration (e.g., tags must be changed); the reader must pay more attention to detail and it's more error prone for the developer.

Answer 4

You can at least safely omit the tags.length > 0 condition as the for loop is a no-op in case of tags.length === 0: (initial t 0 and thus < tags.length even before first run of the loop).

For a purer version of your code, you could convert it to purely functional logic with map etc.

Answer 5

First, the check tags.length > 0 is unneeded. If you try to loop over an empty array the result is a simple no-op.

let arr = []

console.log("before loop")
for (let i = 0; i < arr.length; i++) {
  console.log("inside loop")
}
console.log("after loop")

So, as long as you know you have any array you're safe.

The only thing you need to protect from here is null if the regex doesn't match:

let statement = 'Some text without square brackets inside'
let tags = []
tags = statement.match(/[^[\]]+(?=])/g)

console.log(tags)

An if check is generally sufficient. I'd inverse it because I dislike nesting so you'd have something like this:

if (!tags) {
  return
}
for (let t = 0; t < tags.length; t++) {
    tags[t] = tags[t].toLowerCase()
    tags[t] = tags[t].replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_')
}

However, if you prefer to avoid if checks entirely, you can set always set tags to an empty array if you get null. Then you know you can always handle it the same way. This is easy to achieve using the OR operator (||)

Also worth mentioning that let tags = [] is irrelevant in the code - that value gets immediately overwritten. So you can declare and initialise immediately:

function getTags(statement) {
  let tags = statement.match(/[^[\]]+(?=])/g) || []

  for (let t = 0; t < tags.length; t++) {
    tags[t] = tags[t].toLowerCase()
    tags[t] = tags[t].replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_')
  }
  return tags
}


console.log(getTags('Some text with [square brackets] inside [several] times'))
console.log(getTags('Some text without square brackets inside'))

You can also use the newer nullish coalescing operator (??) which can do the same job:

function getTags(statement) {
  let tags = statement.match(/[^[\]]+(?=])/g) ?? []

  for (let t = 0; t < tags.length; t++) {
    tags[t] = tags[t].toLowerCase()
    tags[t] = tags[t].replace(/[\.,-\/#!$%\^&\*;:{}=\-_`~()]/g, '_')
  }
  return tags
}


console.log(getTags('Some text with [square brackets] inside [several] times'))
console.log(getTags('Some text without square brackets inside'))