-
Notifications
You must be signed in to change notification settings - Fork 298
one sec delay issue solved #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdjusted countdown rollover logic in machine-coding-interview-questions/countdown-timer/src/script.js: when seconds exceed 60, increment minutes and subtract 60 from seconds (previously 59). No other logic or public interfaces changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Timer as Timer Tick
participant State as Time State (min, sec)
Note over Timer,State: Per-second tick
User->>Timer: Start countdown
loop Every second
Timer->>State: Decrement sec
alt sec < 0
State->>State: sec += 60<br/>min -= 1
else sec > 60
Note over State: Adjusting overflow
State->>State: min += 1<br/>sec -= 60
end
Timer-->>User: Render updated mm:ss
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
machine-coding-interview-questions/countdown-timer/src/script.js (1)
33-37
: Minutes normalization can drop time for values ≥ 120; carry hours and remove hard capThe cap to 60 can lose minutes (e.g., 180m → 1h 60m). Normalize via floor/mod and eliminate the cap.
- if (min.value > 60) { - hour.value++; - min.value = parseInt(min.value) - 60; - } - min.value = min.value > 60 ? 60 : min.value; + if (parseInt(min.value, 10) >= 60) { + var m = parseInt(min.value, 10); + hour.value = (parseInt(hour.value || 0, 10) + Math.floor(m / 60)).toString(); + min.value = (m % 60).toString(); + }
🧹 Nitpick comments (3)
machine-coding-interview-questions/countdown-timer/src/script.js (3)
29-33
: Normalize seconds with >= 60 and carry multiple minutesAlso handle inputs like 120s (2 minutes) in one pass and specify radix.
- if (sec.value > 60) { - min.value++; - sec.value = parseInt(sec.value) - 60; - } + if (parseInt(sec.value, 10) >= 60) { + var s = parseInt(sec.value, 10); + min.value = (parseInt(min.value || 0, 10) + Math.floor(s / 60)).toString(); + sec.value = (s % 60).toString(); + }
47-54
: Simplify zero-padding with padStart and explicit parsingReduces coercion surprises and keeps formatting consistent.
- sec.value = `${sec.value <= 10 ? "0" : ""}${sec.value - 1}`; + var sNext = parseInt(sec.value || "0", 10) - 1; + sec.value = String(sNext).padStart(2, "0"); @@ - min.value = `${min.value <= 10 ? "0" : ""}${min.value - 1}`; + var mNext = parseInt(min.value || "0", 10) - 1; + min.value = String(mNext).padStart(2, "0"); @@ - min.value = 60; - hour.value = `${hour.value <= 10 ? "0" : ""}${hour.value - 1}`; + min.value = 60; + var hNext = parseInt(hour.value || "0", 10) - 1; + hour.value = String(hNext).padStart(2, "0");
73-73
: Comment label mismatchThis block ends the Stop button handler, not Start.
- // Start Timer Button - END + // Stop Timer Button - END
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
machine-coding-interview-questions/countdown-timer/src/script.js
(1 hunks)
🔇 Additional comments (1)
machine-coding-interview-questions/countdown-timer/src/script.js (1)
31-31
: Fix off-by-one in seconds rollover — LGTMChanging 59 → 60 correctly normalizes values like 61s → 1m 1s instead of 2s.
Fixes : #15
Where we are formatting the seconds time, I change the value from 59 to 60.
Summary by CodeRabbit