Skip to content

Conversation

@skyturkish
Copy link

@skyturkish skyturkish commented Mar 2, 2024

I hope this message finds you well.

While looking for a library, your repository caught my eye, and I found a place where I think I could help. I believe there could be a better version of the 'src/countdown.tsx' file. With your permission, let me explain why I think so and what I have done as a solution.

Firstly, the four main values(isActive, isInactive, isRunning, isPaused) in the file are a bit confusing and change together every time, which didn't feel quite right to me. Let me give you an example.

const [isActive, setIsActive] = useState(false)
const [isInactive, setIsInactive] = useState(true)

These two lines are simply the opposite of each other; we can use !isActive where we want to use isInactive.

If we make this change, our variable count will decrease from 4 to 3, Good, we've reduced one value, but I believe we can do even better.

The isActive value is actually a value that checks whether the isRunning or isPaused value is true. Then, we do not need the isActive value either. Actually, the four values(isActive, isInactive, isRunning, isPaused) we are export are essentially just an interpretation. Let's just keep a single value and change it, and finally, we can find the other values(isActive, isInactive, isRunning, isPaused) by interpreting this value.

We need just one status and an enum to help us with this matter. Let's refactor the lines where the four values change into a state where only a single value changes, with the help of enums.

After making these changes, when we look at the code, instead of four values changing at the same time, there's only one value, and it's not a value like false or true, but a value we can read like English.

I've run the test "npm run test" all 20 test is successful.

I'm curious about your thoughts on this topic and the changes,

Take care of yourself.

@skyturkish skyturkish force-pushed the refactor-countdown-hook branch from a3a0678 to 221d56a Compare March 2, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant