Skip to content

Please Teach Me#1

Open
rustyneuron01 wants to merge 2 commits into
mainfrom
refactor
Open

Please Teach Me#1
rustyneuron01 wants to merge 2 commits into
mainfrom
refactor

Conversation

@rustyneuron01
Copy link
Copy Markdown
Owner

Please Teach Me

Comment thread src/Components/StopWatch/StopWatch.tsx Outdated
setTime((time) => time + 10);
}, 10);
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not to bad to leave blank here but it doesn't seem good as well.

Comment thread src/Components/StopWatch/StopWatch.tsx Outdated
setTime(0);
interval.current = null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.

Comment thread src/Components/StopWatch/StopWatch.tsx Outdated
function StopWatch() {
const [isActived, setIsActived] = useState(false);
const [isPaused, setIsPaused] = useState(true);
const [isActived, setIsActived] = useState(false); // For Check Press Reset Button
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the variable name is isActive, it actually means whether the timer is active or not.
There's a saying "Code itself is a comment"

Comment thread src/Components/StopWatch/StopWatch.tsx Outdated
const [isActived, setIsActived] = useState(false);
const [isPaused, setIsPaused] = useState(true);
const [isActived, setIsActived] = useState(false); // For Check Press Reset Button
const [isPaused, setIsPaused] = useState(true); // For Check Pause
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. I have no idea why we need isPaused,
We can just have isActive (isActived is a wrong name) state. And if a user click 'Reset', you just need to set isActive as false and timer as zero.

@hungry-engineer
Copy link
Copy Markdown

We're getting closer. Let's fix again.

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.

2 participants