Hi there! How are you? I hope you are well!
This week I came up with a very interesting problem at work. Let’s say that we have to “unlock” something (it is irrelevant what) using a form with a password field. When we input the correct password some background processes run in order to initialize that something… and let us move on to the main screen of the app. Well, I will give you a little piece of code and a very small refactoring we can make to make it a little bit better than it is. We use redux-saga for this kind of processing logic, so here is a very small part of this that (you guessed right, it breaks under certain circumstances…) is dealing with a check for if that something is initialized on the redux store!
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.isIn itialized), '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) } }
I hope I am passing my message in a clear way here. If I need to refactor even the slightest piece of the code such as our example, I write a test that I can rely on to check if the code is still working as before!
I hope you enjoyed it as much as I do writing it… 🙂
Cheers
PS: just food for further thought… How do you test the infinite loop? There is a discussion here but there is not conclusion: https://github.com/redux-saga/redux-saga/issues/862