Kevin Powell - Community•11mo ago

[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

import { useEffect, useState } from "react";
import "./Timer.css";

const Timer = () => {
  const workoutCount = 5.0;
  const [appState, setAppState] = useState({ count: workoutCount, state: "stopped" });
  const [timer, setTimer] = useState(null)
  const cooldownCount = 3;

  //Pure Function
  const newTick = (state, cooldownCount, workoutCount) => {
    if (state.state == "stopped") {
      return { count: workoutCount, state: state.state };
    }

    if (state.count > 0) {
      return { count: Math.max(state.count - 0.05, 0), state: state.state };
    } else {
      // get below 0
      if (state.state == "workout")
        // in workout
        return { count: cooldownCount, state: "cooldown" };
      // in cooldown mode
      else return { count: workoutCount, state: "workout" };
    }
  };

  const startTimer = () => {
    let interval = setInterval(() => {
      const event = new CustomEvent("tick");
      document.dispatchEvent(event)
    }, 50)
    setTimer(interval);
    setAppState({ count: workoutCount, state: "workout" });
  };

  const stopTimer = () => {
    clearInterval(timer);
    setAppState({ count: workoutCount, state: "stopped" });
  };

  const toggleTimer = () => {
    if (appState.state !== "stopped") {
      stopTimer();
    } else {
      startTimer();
    }
  };

  useEffect(() => {
    let listener = document.addEventListener("tick", () => {
      setAppState((prevState) => {
        return newTick(prevState, cooldownCount, workoutCount)
      });
    });
   
    return document.removeEventListener("tick", listener)
  }, []);

import { useEffect, useState } from "react";
import "./Timer.css";

const Timer = () => {
  const workoutCount = 5.0;
  const [appState, setAppState] = useState({ count: workoutCount, state: "stopped" });
  const [timer, setTimer] = useState(null)
  const cooldownCount = 3;

  //Pure Function
  const newTick = (state, cooldownCount, workoutCount) => {
    if (state.state == "stopped") {
      return { count: workoutCount, state: state.state };
    }

    if (state.count > 0) {
      return { count: Math.max(state.count - 0.05, 0), state: state.state };
    } else {
      // get below 0
      if (state.state == "workout")
        // in workout
        return { count: cooldownCount, state: "cooldown" };
      // in cooldown mode
      else return { count: workoutCount, state: "workout" };
    }
  };

  const startTimer = () => {
    let interval = setInterval(() => {
      const event = new CustomEvent("tick");
      document.dispatchEvent(event)
    }, 50)
    setTimer(interval);
    setAppState({ count: workoutCount, state: "workout" });
  };

  const stopTimer = () => {
    clearInterval(timer);
    setAppState({ count: workoutCount, state: "stopped" });
  };

  const toggleTimer = () => {
    if (appState.state !== "stopped") {
      stopTimer();
    } else {
      startTimer();
    }
  };

  useEffect(() => {
    let listener = document.addEventListener("tick", () => {
      setAppState((prevState) => {
        return newTick(prevState, cooldownCount, workoutCount)
      });
    });
   
    return document.removeEventListener("tick", listener)
  }, []);

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

Joao•11mo ago

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.

const APP_STATES = {
  WORKOUT: 'workout',
  COOLDOWN: 'cooldown',
  STOPPED: 'stopped',
};

setAppState({ count: workoutCount, state: APP_STATES.STOPPED });

const APP_STATES = {
  WORKOUT: 'workout',
  COOLDOWN: 'cooldown',
  STOPPED: 'stopped',
};

setAppState({ count: workoutCount, state: APP_STATES.STOPPED });

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:

src/
  components/
    Timer/
      Timer.jsx
      Timer.css
    AnotherComponent/
      AnotherComponent.jsx
      AnotherComponent.css

src/
  components/
    Timer/
      Timer.jsx
      Timer.css
    AnotherComponent/
      AnotherComponent.jsx
      AnotherComponent.css

CDLOP•11mo ago

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

Joao•11mo ago

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.

CDLOP•11mo ago

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

Joao•11mo ago

playing with time always is 😄 I can probably give you a quick example of how it would work, just a sec

Joao•11mo ago

function loop(timestamp) {
  const diff = timestamp - previousTick;
  previousTick = timestamp;
  loopCounter += diff;

  if (loopCounter >= 1000) {
    loopCounter = 0;

    if (playing) {
      decreaseCounter(timeInMs);

      if (timeInMs <= 0) {
        playing = false;
        updateCounterBtn();
      }
    }
  }

  requestAnimationFrame(loop);
}

function loop(timestamp) {
  const diff = timestamp - previousTick;
  previousTick = timestamp;
  loopCounter += diff;

  if (loopCounter >= 1000) {
    loopCounter = 0;

    if (playing) {
      decreaseCounter(timeInMs);

      if (timeInMs <= 0) {
        playing = false;
        updateCounterBtn();
      }
    }
  }

  requestAnimationFrame(loop);
}

This is the basic structure, it's from here: https://codepen.io/D10f/pen/VwQVLNg

D10f

CodePen

Countdown timer with mousepress

...

Joao•11mo ago

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

CDLOP•11mo ago

That's pretty neat! definitely feels a little nicer than the way we had it (even though it's not the exact code)

Joao•11mo ago

Hope it helps!

CDLOP•11mo ago

Very much so, thanks for taking the time to respond 🙂

Gaming

Programming

[React] How does this code look to you?