See all posts

Micro Refactoring

February 23, 2022
1 min

Today while working in the legacy part of our app I came across this code[1]:

useEffect(() => {
  if (prevIsProcessing && !isProcessing && !isError) {
    toggleModal('PostEditModal');
  }
}, [isProcessing]);

Can you guess what it does and why it exists?

The code checks if the post has just been saved: the isProcessing goes from true to false. If the post has been just saved, it will close the post-editing modal.

You may be asking why this is not done inside a callback function. Well, the legacy actions in our app don’t provide a way to detect when they have finished. Because of this, we instead rely on the store state change to detect when they are done (in this case isProcessing).

Although refactoring this component to our new system would improve the codebase even further, that is a much larger task for another day. Today, I simply want to fix a bug. But while I’m here, I might as well improve the codebase a tiny bit for the next reader.

The easy win I opted for was to extract the conditional statement into a variable that provides a better explanation what is going on:

const hasBeenSaved = prevIsProcessing && !isProcessing && !isError;

useEffect(() => {
  if (hasBeenSaved) {
    toggleModal('PostEditModal');
  }
}, [hasBeenSaved]);

Footnotes
  1. This is a simplified version of the code to more easily illustrate the small win that this change brings.
Previous A Short Cautionary Tale About Refactoring Next Lessons Learned: The Passionate Programmer