-
Notifications
You must be signed in to change notification settings - Fork 6
Added docker compose file for Mongo + Redis and Optimized helper.go #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: dev
Are you sure you want to change the base?
Conversation
Added health checks for redis and mongodb services.
Removed environment variables from service configuration.
| }) | ||
| for i, part := range parts { | ||
| parts[i] = cases.Title(language.English).String(part) | ||
| if !strings.ContainsAny(s, "_-") { |
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 the previous Casebuilder above and used strings.Builder instead. Using Builder here to avoid allocations from Join.
| if strings.EqualFold(r, rarity) { | ||
| return i | ||
| } | ||
| if idx, ok := rarityMap[strings.ToLower(rarity)]; ok { |
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 lookup here instead of loop
| return re.ReplaceAllStringFunc(template, func(match string) string { | ||
| name := strings.Trim(match, "{}") | ||
| return variableRegex.ReplaceAllStringFunc(template, func(match string) string { | ||
| name := match[1 : len(match)-1] // Faster than strings.Trim |
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.
See first comment about pre compiling regex.
|
|
||
| func Filter[T any](slice []T, predicate func(T) bool) []T { | ||
| var result []T | ||
| result := make([]T, 0, len(slice)/2) // Estimate half will match |
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.
Pre allocating result slice here, no need to start with nil and append.
| } | ||
| } | ||
|
|
||
| sort.Slice(slice, func(i, j int) bool { |
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 quicksort over bubblesort here because n log n on top 😈
| return 1 | ||
| } | ||
|
|
||
| func GetHexColor(color string) string { |
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.
fmt.sscaf is slow, strconv is faster here.
WarpWing
left a 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.
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 adds Docker Compose configuration for MongoDB and Redis services and implements significant performance optimizations in helper.go, including pre-compiled regexes, buffer pooling, improved string building, and replacing bubble sort implementations with Go's built-in sort package.
Key changes:
- Added Docker Compose file with MongoDB and Redis service configurations including health checks
- Optimized helper functions with better algorithms (replacing O(n²) bubble sorts with O(n log n) sorts)
- Improved memory efficiency through string builders, buffer pooling, and pre-allocated maps
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| docker-compose.yml | Adds containerized MongoDB and Redis services with health checks and persistent volumes |
| src/utility/helper.go | Performance optimizations including regex pre-compilation, buffer pooling, efficient sorting, and improved string handling |
| NotEnoughUpdates-REPO | Updates subproject commit reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| func Filter[T any](slice []T, predicate func(T) bool) []T { | ||
| var result []T | ||
| result := make([]T, 0, len(slice)/2) // Estimate half will match |
Copilot
AI
Dec 31, 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 capacity pre-allocation assumes half the items will match, which may waste memory if the actual match rate is much lower, or cause reallocations if it's higher. Consider using len(slice) as the capacity to avoid reallocations in worst-case scenarios, or use 0 to let Go's append grow naturally for typical cases.
| result := make([]T, 0, len(slice)/2) // Estimate half will match | |
| result := make([]T, 0, len(slice)) // Preallocate for worst-case: all elements match |
| return re.ReplaceAllStringFunc(template, func(match string) string { | ||
| name := strings.Trim(match, "{}") | ||
| return variableRegex.ReplaceAllStringFunc(template, func(match string) string { | ||
| name := match[1 : len(match)-1] // Faster than strings.Trim |
Copilot
AI
Dec 31, 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.
While the inline comment explains the optimization, it would be clearer to note that this assumes the regex match format is always {name}. Consider adding a brief explanation of why this is safe given the regex pattern.
| name := match[1 : len(match)-1] // Faster than strings.Trim | |
| name := match[1 : len(match)-1] // Assumes match is "{name}" as enforced by variableRegex (`\{(\w+)\}`); faster than strings.Trim |
| if intValue := int(value); intValue > 0 { | ||
| return "+" + strconv.Itoa(intValue) |
Copilot
AI
Dec 31, 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.
Converting float64 to int by direct cast truncates rather than rounds, which can produce incorrect results for values like 9.9 (becomes 9 instead of 10). Consider using math.Round(value) before the int conversion to ensure proper rounding behavior.
| } | ||
|
|
||
| return fmt.Sprintf("%.0f", math.Abs(value)) | ||
| return strconv.Itoa(int(math.Abs(value))) |
Copilot
AI
Dec 31, 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.
Similar to the previous issue, direct casting from float64 to int truncates instead of rounding. Use int(math.Round(math.Abs(value))) to ensure values like -5.9 round to 6 instead of truncating to 5.
| return strconv.Itoa(int(math.Abs(value))) | |
| return strconv.Itoa(int(math.Round(math.Abs(value)))) |
| if httpErr != nil { | ||
| return | ||
| } | ||
| resp.Body.Close() |
Copilot
AI
Dec 31, 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 response body is closed without checking for errors or using defer. If the function returns early before this line, the body won't be closed, causing a resource leak. Use defer resp.Body.Close() immediately after checking the HTTP error to ensure the body is always closed.
| resp.Body.Close() | |
| defer resp.Body.Close() |

No description provided.