-
-
Notifications
You must be signed in to change notification settings - Fork 720
Add solution for Challenge 17 by Johrespi #731
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
Conversation
WalkthroughAdds a Go solution implementing a palindrome checker function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 2
🧹 Nitpick comments (1)
challenge-17/submissions/Johrespi/solution-template.go (1)
27-27: Remove trailing whitespace.Lines 27 and 39 contain trailing whitespace. While not functionally problematic, removing it improves code hygiene.
Also applies to: 39-39
| func main() { | ||
| // Get input from the user | ||
| var input string | ||
| fmt.Print("Enter a string to check if it's a palindrome: ") | ||
| fmt.Scanln(&input) | ||
|
|
||
| // Call the IsPalindrome function and print the result | ||
| result := IsPalindrome(input) | ||
| if result { | ||
| fmt.Println("The string is a palindrome.") | ||
| } else { | ||
| fmt.Println("The string is not a palindrome.") | ||
| } | ||
| } |
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.
Fix input reading to support multi-word palindromes.
fmt.Scanln stops at the first whitespace, so multi-word palindrome test cases like "A man a plan a canal Panama" will only read "A". This significantly limits the usability of the interactive tester.
Apply this diff to read the entire line:
import (
+ "bufio"
"fmt"
+ "os"
"strings"
"unicode"
)
func main() {
- // Get input from the user
- var input string
+ reader := bufio.NewReader(os.Stdin)
fmt.Print("Enter a string to check if it's a palindrome: ")
- fmt.Scanln(&input)
+ input, _ := reader.ReadString('\n')
+ input = strings.TrimSpace(input)
// Call the IsPalindrome function and print the result
result := IsPalindrome(input)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In challenge-17/submissions/Johrespi/solution-template.go around lines 9 to 22,
the interactive input uses fmt.Scanln which stops at the first whitespace;
replace it with reading the full line (e.g., use
bufio.NewReader(os.Stdin).ReadString('\n')), then trim the trailing
newline/whitespace with strings.TrimSpace before passing to IsPalindrome, and
update imports to include bufio, os and strings as needed.
| n := len(cleanedStr) | ||
|
|
||
| for i := 0; i < n/2; i++ { | ||
| if cleanedStr[i] != cleanedStr[n-1-i] { | ||
| return false | ||
| } | ||
| } |
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.
Fix byte-level comparison that breaks for non-ASCII Unicode characters.
The current implementation compares individual bytes via cleanedStr[i], which fails for multi-byte UTF-8 characters. Since the function accepts Unicode letters (via unicode.IsLetter), palindromes with non-ASCII characters like "été" will incorrectly return false.
Example failure:
- Input: "été" (French, a valid palindrome)
- UTF-8 bytes: [0xC3, 0xA9, 0x74, 0xC3, 0xA9]
- Comparison:
cleanedStr[0](0xC3) ≠cleanedStr[4](0xA9) → false ❌
Apply this diff to compare runes instead of bytes:
cleanedStr := cleaned.String()
-
- n := len(cleanedStr)
+
+ runes := []rune(cleanedStr)
+ n := len(runes)
for i := 0; i < n/2; i++ {
- if cleanedStr[i] != cleanedStr[n-1-i] {
+ if runes[i] != runes[n-1-i] {
return false
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| n := len(cleanedStr) | |
| for i := 0; i < n/2; i++ { | |
| if cleanedStr[i] != cleanedStr[n-1-i] { | |
| return false | |
| } | |
| } | |
| runes := []rune(cleanedStr) | |
| n := len(runes) | |
| for i := 0; i < n/2; i++ { | |
| if runes[i] != runes[n-1-i] { | |
| return false | |
| } | |
| } |
🤖 Prompt for AI Agents
In challenge-17/submissions/Johrespi/solution-template.go around lines 40 to 46,
the code compares bytes (cleanedStr[i]) which breaks for multi-byte UTF-8
characters; change the logic to operate on runes by converting cleanedStr to a
[]rune (or building a rune slice while filtering) and then compare runes by
index (runes[i] vs runes[n-1-i]) using the rune-length n, so multi-byte Unicode
letters are handled correctly.
Challenge 17 Solution
Submitted by: @Johrespi
Challenge: Challenge 17
Description
This PR contains my solution for Challenge 17.
Changes
challenge-17/submissions/Johrespi/solution-template.goTesting
Thank you for reviewing my submission! 🚀