[React] How does this code look to you?
Friend and I spent a few hours yesterday trying to get setInterval and useEffect to work on our little workout app. Brains were fried, and it works, but we're curious on your opinions on how we 've done it.
Can't copy paste the code, but everything is inside the Timer.jsx file.
https://github.com/callum-laing/react-playground/blob/main/src/components/Timer.jsx
GitHub
react-playground/src/components/Timer.jsx at main ยท callum-laing/re...
Contribute to callum-laing/react-playground development by creating an account on GitHub.
10 Replies
That's not too bad actually. A bit messy but could be clean up easily by moving the timer onto a separate hook. I also really like that you are using toLocaleString to output the timer, instead of toFixed, very nice touch.
As I said I would move the timer to an external file to clean things up (also makes it reusable if you want to have multiple timers running at once). And instead of using a custom event and listening for ticks every 50ms you can use requestAnimationFrame. It'll run more smoothly and you it's designed for things like that. Although the use of a custom event and listening for it it perfectly valid, but for something that runs so often I like requestAnimationFrame better.
I'm also not a big fan of combining non-related pieces of information under the same call of useState. Here,
count
and state
... mmm, well, I guess you could argue they are related, and you are modifying them at once so it kinda makes sense but it's something to consider though this is mostly preference.
Try to avoid magic numbers as well: count: Math.max(state.count - 0.05, 0)
what is this supposed to represent? It's not immediately obvious so maybe move that onto a variable with a descriptive name instead.
For extra tidiness you should consider moving the possible states of the app onto a separate variable, making it an enum.
This is self-documenting so it helps readability and understanding how the app behaves. But also, helps to avoid silly typos like stopepd that would make it annoying to correct, plus it helps code editors provide you with autocompletion.
As for code structure, it'd be better to move the components into its own folder and include the js and css files there. This is not a problem for a single component, but when you have 10, 20, 30... you'd have twice that amount of files. So, under components:
All valid feedback, thanks @Joao . I think after a few hours we just wanted the darn thing to work, so the mess is uhh... yeah sure, haha!
I'm new to React, and my friend codes in C#/F# so it's new to both of us, we weren't aware of requestAnimationFrame, that's a neat alternative for sure
For sure make it work first, that's the most important part!
setInterval will do just fine, so don't worry about it for now. My main focus would be on making the timer into a hook to make things cleaner.
setInterval is when all hell broke loose LOL, it was easy up until that point, then it turned into a bit of a clown fiesta
playing with time always is ๐
I can probably give you a quick example of how it would work, just a sec
This is the basic structure, it's from here: https://codepen.io/D10f/pen/VwQVLNg
It's basically a recursive function that calls itself as fast as the refresh rate of the screen allows it (making it ideal for smooth animations as the name suggests, also games). You can control how often the actual counter updates by dividing by 1000 (every second) 100 (every hundredth), etc.
And here's a raw implementation in React. It's not on separate files because of Codepen, but ideally that's how you'd structure.
https://codepen.io/D10f/pen/JjzWdYB
That's pretty neat! definitely feels a little nicer than the way we had it (even though it's not the exact code)
Hope it helps!
Very much so, thanks for taking the time to respond ๐