-
Notifications
You must be signed in to change notification settings - Fork 35
Submission: Siwach Toprasert Coding Challenge #25
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: main
Are you sure you want to change the base?
Conversation
…LUTIONS.md to further explain the reasons and approach
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.
Pull Request Overview
This PR implements multiple coding challenges including SQL queries for pledge campaigns, a concurrent file aggregator, fixes to thread-unsafe counter implementations, and run‑length encoding in both Go and C#.
- Updated SQL queries with window functions and index improvements
- Added a multithreaded file aggregator with per-file timeouts
- Fixed race conditions in counter functions and provided run‑length encoding implementations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/04-sql-reasoning/csharp/Queries.cs | Added SQL queries for campaign summaries and 90th percentile calculations with appropriate indexes |
| tasks/03-sync-aggregator/csharp/Aggregator.cs | Implemented a multithreaded aggregator with job queuing and per-file timeout logic |
| tasks/02-fix-the-bug/go/buggy_counter.go | Replaced manual counter increments with an atomic operation to prevent race conditions |
| tasks/02-fix-the-bug/csharp/BuggyLib/BuggyCounter.cs | Replaced manual counter increments with Interlocked.Increment for thread safety |
| tasks/01-run-length/go/rle.go | Implemented run‑length encoding, but potential Unicode handling issues exist in multi‑byte inputs |
| tasks/01-run-length/csharp/Rle.cs | Implemented Unicode‑safe run‑length encoder using TextElementEnumerator |
| SOLUTIONS.md | Documented the overall approach and rationale for each task |
| if (!finished) | ||
| { | ||
| thread.Interrupt(); | ||
| Console.WriteLine($"Timed out: {relPath}"); | ||
| return new Result(relPath, 0, 0, "timeout"); | ||
| } | ||
|
|
Copilot
AI
Jun 19, 2025
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.
The check for '!finished' on line 119 is redundant because the unfinished condition was already handled at line 108. Removing the duplicate check will simplify the control flow.
| if (!finished) | |
| { | |
| thread.Interrupt(); | |
| Console.WriteLine($"Timed out: {relPath}"); | |
| return new Result(relPath, 0, 0, "timeout"); | |
| } |
| prev := rune(s[0]) | ||
| for _, curr := range s[1:] { |
Copilot
AI
Jun 19, 2025
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.
Using s[0] to initialize the first rune may produce incorrect results for multi-byte Unicode characters. Consider iterating over the entire string using a range loop to correctly decode Unicode code points.
| prev := rune(s[0]) | |
| for _, curr := range s[1:] { | |
| var prev rune | |
| for i, curr := range s { | |
| if i == 0 { | |
| prev = curr | |
| continue | |
| } |
No description provided.