finish #20
finish #20
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR completes the exercise by providing working implementations for each task and documenting the approaches in SOLUTION.md.
- Implements SQL queries and index definitions for campaign pledges (Task 04).
- Adds a concurrent file stats aggregator with per-file timeouts (Task 03).
- Fixes the counter race and adjusts ID generation (Task 02).
- Implements run-length encoding for UTF-8 strings (Task 01) and adds solution notes.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/04-sql-reasoning/go/queries.go | Added SQLA/SQLB queries and index definitions |
| tasks/03-sync-aggregator/go/aggregator.go | Implemented Aggregate, worker pool, and timeout helpers |
| tasks/02-fix-the-bug/go/buggy_counter.go | Introduced mutex and changed NextID logic |
| tasks/01-run-length/go/rle.go | Added Encode function body for run-length encoding |
| SOLUTION.md | Added solution notes detailing languages, approaches, time |
| `, | ||
| ` | ||
| CREATE INDEX idx_pledge_donor_id | ||
| ON pledge (campaign_id); |
There was a problem hiding this comment.
The idx_pledge_donor_id index is created on campaign_id instead of donor_id, so it won’t accelerate donor-based lookups. Change it to ON pledge (donor_id).
| ON pledge (campaign_id); | |
| ON pledge (donor_id); |
|
|
||
| select { | ||
| case res := <-resultChan: | ||
| return res |
There was a problem hiding this comment.
The select in processFileWithTimeout never handles a timeout case, so timeoutSec is ignored and this can block indefinitely. Add a case <-time.After(time.Duration(timeoutSec)*time.Second): branch to return a timeout status.
| return res | |
| return res | |
| case <-time.After(time.Duration(timeoutSec) * time.Second): | |
| return Result{ | |
| Path: path, | |
| Lines: 0, | |
| Words: 0, | |
| Status: "timeout", | |
| } |
| mu.Lock() | ||
| defer mu.Unlock() | ||
|
|
||
| id := current + int64(rand.Int()) |
There was a problem hiding this comment.
Using rand.Int() here introduces unpredictable and potentially duplicate IDs. If sequential unique IDs are intended, simply return current (or use atomic.AddInt64) under the lock without adding randomness.
| id := current + int64(rand.Int()) | |
| id := current |
| collector := make(map[string]int) | ||
| order := make([]string, 0) | ||
| for _, c := range s { | ||
| char := string(c) | ||
| if val, ok := collector[char]; ok { | ||
| collector[char] = val + 1 | ||
| continue | ||
| } | ||
| collector[char] = 1 | ||
| order = append(order, char) | ||
| } | ||
|
|
||
| ans := "" | ||
| for _, w := range order { | ||
| ans = fmt.Sprintf("%s%s%d", ans, w, collector[w]) | ||
| } | ||
|
|
There was a problem hiding this comment.
This approach counts total occurrences rather than consecutive runs, so it fails when characters repeat non-adjacently. Update the algorithm to track and emit run lengths as you iterate through s.
| collector := make(map[string]int) | |
| order := make([]string, 0) | |
| for _, c := range s { | |
| char := string(c) | |
| if val, ok := collector[char]; ok { | |
| collector[char] = val + 1 | |
| continue | |
| } | |
| collector[char] = 1 | |
| order = append(order, char) | |
| } | |
| ans := "" | |
| for _, w := range order { | |
| ans = fmt.Sprintf("%s%s%d", ans, w, collector[w]) | |
| } | |
| if len(s) == 0 { | |
| return "" | |
| } | |
| ans := "" | |
| currentChar := string(s[0]) | |
| count := 1 | |
| for i := 1; i < len(s); i++ { | |
| char := string(s[i]) | |
| if char == currentChar { | |
| count++ | |
| } else { | |
| ans = fmt.Sprintf("%s%s%d", ans, currentChar, count) | |
| currentChar = char | |
| count = 1 | |
| } | |
| } | |
| // Append the last run | |
| ans = fmt.Sprintf("%s%s%d", ans, currentChar, count) |
|
|
||
| - Language: Go | ||
| - Approach: ~~Output is only want to avoid the duplication so I decided to keep it simple for reduce time for other problem~~ After trying to not use mutex so I already got it that I misunderstand the problem | ||
| - Why: Should use realible library to generate the uuid instead if id not necessary to order and need to use mutex to lock the accessibility of worker |
There was a problem hiding this comment.
[nitpick] Spelling/grammar: “realible” should be “reliable”, and the sentence could be clarified. For example: “Use a reliable UUID library when ordering isn’t required, and protect counter access with a mutex.”
| - Why: Should use realible library to generate the uuid instead if id not necessary to order and need to use mutex to lock the accessibility of worker | |
| - Why: Use a reliable UUID library when ordering isn’t required, and protect counter access with a mutex to ensure thread safety. |
3 participants
No description provided.