Refactoring a generator

function* waitForSthToInit() {
  while (true) {
    const isInitialized = yield select(selectors.isInitialized)
    if (isInitialized) {
      break
    }
    yield call(delay, 100)
  }
}

Ok, that’s the generator function. WARNING! There are no tests in the file for any of the existing sagas. So, BEFORE we refactor let’s write a test to make sure we do not change the external behavior of the code. We want it to keep doing the same stuff it was doing before our refactoring.

So, here is a small test to check if we exit the generator loop successfully. This means when the isInitialized flag is set to true after reading from the store! Testing a saga we will focus on the order of execution, so the test will be strictly like this:

it('should terminate if something at some point gets initialized', () => {
  const generator = cloneableGenerator(waitForSthToInit)()
  assert.deepEqual(
    generator.next().value,
    select(selectors.isInitialized),
    'should check if something is initialized'
  )

  assert.deepEqual(
    generator.next().value,
    call(delay, 100),
    'should delay for 100ms'
  )

  assert.deepEqual(
    generator.next().value,
    select(selectors.isInitialized),
    'should check if something is initialized'
  )

  // I hate comments but this is for illustration purpose:
  // we pass true here to .next() as the value of the previous
  // yield, that is the isInitialized selector's value
  assert.deepEqual(
    generator.next(true),
    { done: true, value: undefined },
    'should be done'
  )
})

You will wonder, what happens if that something does not get initialized ever??? Then we go to an infinite loop…. Nice, huh? And guess also that this happened: the code in order to initialize that something passes through a lot of functions and modules and at some point one of them blocked execution! So, we never could run the rest of the code from that point afterwards… We fixed the blocking but now we have a more design-like decision to make? Do we really need the loop? But that is a topic for some other refactoring. At this blog post, we are going to focus only on a tiny improvement of the code. You can see that we create a new const variable each time. Why don’t we use let instead and make just one variable that changes value, not itself?

And now the question! Would you take the risk of a refactoring small as this without having a solid test in place? I would not even if they paid me 1.000.000 dollars to do it as a bribe!! Why is that? It is the fear of the infinite loop… right? Why rely on intuition or even good knowledge of the language and not just ask the computer with a test instead? At least, this way I know that I can blame the PC for blowing up the app… 😛

Refactoring const to let:

export function* waitForSthToInit() {
  let isInitialized
  while (true) {
    isInitialized = yield select(selectors.isInitialized)
    if (isInitialized) {
      break
    }
    yield call(delay, 100)
  }
}