test: playground code for PR comment testing#218
Conversation
Sample functions with deliberate review opportunities: missing error handling in Divide, inefficient Fibonacci, loop-based Multiply. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7 files with various issues: regex recompilation, memory leaks, overflow bugs, shadowed builtins, missing edge case handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Overall this looks like a good start, but there are several issues that need addressing before merge. See my inline comments below. |
|
I'd also suggest adding table-driven tests for the math functions. The current test coverage is thin. |
|
One more thing: the |
| } | ||
|
|
||
| func Divide(a, b int) int { | ||
| return a / b |
There was a problem hiding this comment.
Bug: this will panic on division by zero. Add a guard: if b == 0 { return 0 } or return an error.
| func Fibonacci(n int) int { | ||
| if n <= 1 { | ||
| return n | ||
| } |
There was a problem hiding this comment.
Performance: this naive recursive Fibonacci is O(2^n). Use memoization or an iterative approach. This will be unusably slow for n > 30.
| } | ||
|
|
||
| func Multiply(a, b int) int { | ||
| result := 0 |
There was a problem hiding this comment.
Nit: why use a loop here instead of just return a * b? This also breaks for negative b.
|
|
||
| // TODO: this compiles the regex every call, should be a package-level var | ||
| func IsEmail(s string) bool { | ||
| re := regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) |
There was a problem hiding this comment.
Performance: regexp.MustCompile is called every invocation. Move this to a package-level var emailRe = regexp.MustCompile(...).
| } | ||
|
|
||
| // this shadows builtin max, probably bad | ||
| func max2(a, b int) int { |
There was a problem hiding this comment.
This shadows the builtin max (Go 1.21+). Either use the builtin directly or pick a different name. Also this function appears unused.
| if r >= 'A' && r <= 'Z' { | ||
| if i > 0 { | ||
| result.WriteByte('_') | ||
| } |
There was a problem hiding this comment.
The REVIEW comment nails it: HTTPServer becomes h_t_t_p_server. Consider tracking runs of uppercase characters.
| } | ||
|
|
||
| func FahrenheitToCelsius(f float64) float64 { | ||
| return (f - 32) * 5 / 9 |
There was a problem hiding this comment.
Negative bytes will produce confusing output. Add a guard or use Abs.
|
|
||
| import "fmt" | ||
|
|
||
| func Greet(name string) string { |
There was a problem hiding this comment.
Nit: exported functions should have godoc comments.
| return s == Reverse(s) | ||
| } | ||
|
|
||
| func CountWords(s string) int { |
There was a problem hiding this comment.
Nit: CountWords could be WordCount to match Go's noun-first convention (like len, cap).
| } | ||
|
|
||
| func Truncate(s string, maxLen int) string { | ||
| if len(s) <= maxLen { |
There was a problem hiding this comment.
Nit: len(s) counts bytes, not runes. Truncating at a byte boundary could split a multi-byte character. Use []rune(s) for correctness.
| @@ -0,0 +1,37 @@ | |||
| package example | |||
|
|
|||
| type Stack struct { | |||
There was a problem hiding this comment.
Nit: consider using generics (Stack[T any]) instead of any for type safety.
| @@ -0,0 +1,48 @@ | |||
| package example | |||
There was a problem hiding this comment.
Nit: only playground.go has tests. The other 6 files have zero test coverage.
| } | ||
|
|
||
| func CSVToSlice(csv string) []string { | ||
| if csv == "" { |
There was a problem hiding this comment.
Nit: this does not trim whitespace. "a, b, c" returns [" b", " c"]. Consider strings.TrimSpace on each element.
1 participant
Summary
Sample code with deliberate review opportunities for testing PR comment features.
Do not merge — this is a test PR for exercising comment UIs.