Get rid of code smells.
One of my tasks for this sprint was to implement a new feature in a very complicated form, with dynamic questions and a logic to jump between them. I estimated 5 hours for this task, the only change I need to do, was to add an extra validation at the moment to send.
I was still thinking this until I saw the component I need to update. This component has more than 300 lines of code, there was a giant piece of JSX markup that took like 150 lines, and the logic…, there were many setState in this component and useEffects with many dependencies.
I was surprised and also ashamed because I wrote this code, but it turns out into a giant monster that I don't understand at all. You know, like those times when you are staring at your screen for fifteen minutes thinking what on earth this function was for.
Code smells
You might hear this before, if you don't, code smells are practically what I describe before. It indicates weakness in your code, they are not bugs, because the web app works fine, but it can increase the risk of bugs or failures in the future.
So, how do you deal with a code like that?
I followed these tips to refactor the component I describe before, and I hope it could help you too.
useReducer instead of useState
I mentioned that this component has multiple useState
, this was difficult to read because I have to look for all the setState
to know which part of the component updates a certain state.
Yeah, I know, it looks bad. If you end with a code like this, you must consider doing a refactor. Many of these states where related, so I can create a reducer to handle the possible scenarios for this form.
This also increases your knowledge of the effects that these reducer actions have. When you separate your logic in actions you are aware of what action affects certain fields of the state. This gives you more control of the state, and implementing a new feature is easier.
Separate logic.
In this form, I need to fetch some data from an API on graphql. I use apollo and @apollo/react-hooks
. When I get the data I need to update some state. Instead of having the data fetching in the same component, I can create a custom hook that returns just the state I need.
If I need to use a mutation or add an extra query, I could write that logic in this hook also.
I recommend you to follow the separation of concerns principle for data fetching, you can create a separate file for the logic related to the data and just return the state that will be rendered in your component. This could also apply for your UI, creating a presentational component when it's needed will let you understand better how your app works.
Split it into smaller pieces
The UI of the monster component I wrote has more than 100 lines of code. There were div
after div
and it took me more than two minutes to know what exactly I was doing. In all this div
universe there were conditional renders, and I literally have to scroll down for a while to know where this end.
Look at the code above. That currentForm
boolean it's wrapping a big piece of JSX markup. It could be more readable if we move this UI into another component.
So much better, I can split this big component into smaller ones, so the next person that comes, and sees this code would know exactly where is everything.
Conclusion
There is no perfect way of writing code, different teams follow different patterns, but if you feel the way I feel when I saw that code, it probably needs a refactor too. Keep in mind:
If you are using multiple
useState
, try to change it touseReducer
. You'll have better control and a wide vision of what is happening to your state in every action.Separate logic that can be reused. You can create a custom hook for data fetching or create util functions that can be used on other parts of your app.
Split your component into smaller pieces. Extract pieces of JSX markup into a different component to improve the readability of your code.
Top comments (10)
You didn't mention it in this article, so I gotta ask: Were you previously using Redux before using Hooks/useState? I'm asking because I've seen other articles (here and on other sites) with a similar message about
useReducer()
. They show some functional logic withuseState()
, they mention how "ugly" the code looks, then they refactor it to useuseReducer()
and they seem to be quite satisfied with the (supposedly) new-and-improved look.This comment isn't meant to be "anti"-
useReducer()
.useReducer()
is a tool. If you're more comfortable using that tool, then great! But for the life of me, I can't look at the difference in those two examples (or in others I've seen) and fathom why theuseState()
example is "bad" or theuseReducer()
example is "good".Whether it's stated in the narrative or not, I usually find that the implication in these articles comes down to: "I'm accustomed to using Redux. And that's why I think that the code looks so much better with
useReducer()
."If this isn't true for you, then I apologize for the presumption. Even if it is true for you, this isn't meant as a critique on your article or your examples. You did a fine job. I'm just honestly trying to find out whether people who aren't already familiar with Redux are truly seeing
useReducer()
as such a "better" or "cleaner" alternative touseState()
. Or... (as I suspect), the former Redux users are the ones that are more enamored with theuseReducer()
approach.Hi, I don't know if it looks like but I'm not familiar with Redux, I think I used it just two times in my two years working as a developer.
I know what you mean when you said that you can't look at the difference in those examples, but I didn't mean that
useState
code just look ugly anduseReducer
is better, it's just that for this component, in particular, having multiplesetState
just didn't work. It's not maintainable.And this doesn't mean that I'm going to use
useReducer
for all my components. I'm usinguseState
90% of the time, and that will not change, but when you have a complicated state, you may need a reducer.Adding actions can increase your vision of what's happening with all your states. But also you can have a very clean and readable code with just using
setState
.Ahh, very interesting. Thank you for the reply (and I agree with you).
Have you ever tried immerjs for useReducer? It's an extra dependency but I feel it's made my Redux reducers much much more readable and manageable. (Destructuring can become a PITA with enough nested structures).
Btw, saludos desde El Salvador. :)
Hi, I heard of immer.js but I haven't tried it yet. I'll give it a chance in my next project, thanks for the info :)
Nice article !!! keep it up !!!!
Nice!🔥
Great piece!
Mark as a reading list to read later, thanks!
Hi, Elizabeth. Wanted to ask if there are too many useStates then why didn't you try to convert the component into a class component?