[Concept] Regular Expressions-add exercise templates #141
Conversation
WalkthroughAdds a new "Regular Expressions" exercise (slug Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Test as "go test"
participant Template as "templates/41_regex"
participant Solution as "solutions/41_regex"
note over Dev,Test: Run unit tests for regex exercise
Test->>Template: invoke IsValidEmail/ExtractNumbers/ReplaceVowels/IsPhoneNumber
alt template unimplemented
Template-->>Test: zero/default values (fail)
else template implemented / solution present
Solution-->>Test: computed results (pass/fail)
end
note right of Test: Assertions compare results to expected cases
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
zhravan
left a comment
zhravan
left a comment
There was a problem hiding this comment.
@aryasoni98: Can you make changes to file count value as per recent changes please??
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exercises/templates/35_regex/regex_test.go (1)
27-52: Consider adding test case for trailing decimal point.The tests are well-structured, but consider adding a test case for numbers with trailing decimal points (e.g., "123.") to clarify expected behavior. The current regex pattern
\d+\.?\d*in the solution would match "123." as a valid number, which may or may not be intended.Example test case to add:
{"12.34.56", []string{"12.34", "56"}}, + {"Price: 99.", []string{"99."}}, // or expect {"99"} if trailing dots should be excluded {"", []string{}},internal/exercises/solutions/35_regex/regex.go (1)
13-21: Consider refining the number extraction pattern.The pattern
\d+\.?\d*works for the current test cases but has a subtle edge case: it matches numbers with trailing decimal points (e.g., "99." from "Price is 99."). While this passes all current tests, consider whether this behavior is intended.If trailing decimals should be excluded, consider this alternative pattern:
- pattern := `\d+\.?\d*` + pattern := `\d+(\.\d+)?`This requires at least one digit after the decimal point if present, matching: "123", "123.45" but not "123."
However, if the current behavior is acceptable, the implementation is correct as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/exercises.md(1 hunks)internal/exercises/catalog.yaml(1 hunks)internal/exercises/solutions/35_regex/regex.go(1 hunks)internal/exercises/solutions/35_regex/regex_test.go(1 hunks)internal/exercises/templates/35_regex/regex.go(1 hunks)internal/exercises/templates/35_regex/regex_test.go(1 hunks)
🔇 Additional comments (12)
internal/exercises/catalog.yaml (2)
184-192: Inconsistency with AI summary.The AI summary states that the XML concept (slug 37_xml) was removed, but it's still present in the catalog at lines 184-192. This indicates the summary is inconsistent with the actual changes.
172-183: LGTM! Well-structured regex exercise with helpful hints.The new 35_regex concept is well-defined with practical hints covering the key regexp methods (MustCompile, MatchString, FindAllString, ReplaceAllString) and example patterns. The YAML escaping for the regex patterns is correct (e.g.,
\\becomes\in the actual patterns).docs/exercises.md (1)
52-61: LGTM! Documentation correctly reflects the new exercise.The documentation accurately lists the new 35_regex exercise among the advanced topics and maintains proper ordering.
internal/exercises/templates/35_regex/regex_test.go (3)
5-25: LGTM! Comprehensive email validation tests.The test cases cover valid emails, missing components (@, domain, TLD), and edge cases like empty strings. Good coverage for email validation.
54-74: LGTM! Thorough vowel replacement tests.Excellent test coverage including uppercase, lowercase, mixed case, no vowels, empty strings, and all vowels cases.
76-99: LGTM! Comprehensive phone number validation tests.Great test coverage with 10 test cases covering the valid format, missing spaces, missing parentheses, wrong digit counts, and edge cases. This thoroughly validates the strict (XXX) XXX-XXXX format.
internal/exercises/solutions/35_regex/regex_test.go (1)
1-99: LGTM! Tests correctly mirror the template tests.The solution tests are appropriately identical to the template tests, ensuring students and the reference implementation are validated against the same criteria. Same optional improvement suggestion applies here as in the template tests: consider adding a test case for numbers with trailing decimal points to clarify expected behavior.
internal/exercises/templates/35_regex/regex.go (1)
1-37: LGTM! Well-structured exercise template.The template provides clear task descriptions, helpful TODO comments pointing to the required regexp methods, and appropriate placeholder returns. The structure guides students effectively without revealing the implementation.
internal/exercises/solutions/35_regex/regex.go (4)
6-10: LGTM! Robust email validation pattern.The email regex pattern correctly validates standard email formats. The pattern requires:
- One or more valid username characters:
[a-zA-Z0-9._%+-]+- @ symbol
- Domain with at least one dot:
[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}This matches all test cases appropriately.
17-20: LGTM! Proper nil handling for empty results.Good defensive programming to convert nil slice to empty slice, ensuring consistent return value semantics and preventing potential nil pointer issues in calling code.
24-28: LGTM! Clean vowel replacement implementation.The pattern correctly matches all vowels (both cases) and the ReplaceAllString method efficiently replaces them with asterisks. Simple and effective.
31-35: LGTM! Precise phone number validation.The pattern correctly enforces the strict (XXX) XXX-XXXX format with:
- Escaped parentheses:
\(\d{3}\)- Space separator
- Three digits, hyphen, four digits:
\d{3}-\d{4}- Anchors ensure exact match:
^...$This validates all test cases accurately.
|
@aryasoni98 : Can you resolve the conflicts? |
…g regex exercise to 41_regex
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exercises/solutions/41_regex/regex.go (2)
12-21: Consider refining the number extraction pattern.The current pattern
\d+\.?\d*works for the given test cases but could match edge cases like "123." (number with trailing dot but no fractional part).Consider using
\d+(?:\.\d+)?instead, which explicitly requires digits after the dot when present.Apply this diff to refine the pattern:
func ExtractNumbers(text string) []string { - pattern := `\d+\.?\d*` + pattern := `\d+(?:\.\d+)?` re := regexp.MustCompile(pattern) matches := re.FindAllString(text, -1) if matches == nil {Note: The nil check for empty results is good practice and ensures consistent return behavior.
1-35: Optional: Consider compiling regexes at package level for better performance.Currently,
regexp.MustCompileis called on every function invocation. For better performance in production code, consider compiling the patterns once at package level:var ( emailPattern = regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) numberPattern = regexp.MustCompile(`\d+(?:\.\d+)?`) vowelPattern = regexp.MustCompile(`[aeiouAEIOU]`) phonePattern = regexp.MustCompile(`^\(\d{3}\) \d{3}-\d{4}$`) )However, the current implementation is perfectly fine for an educational exercise where clarity is more important than micro-optimizations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/exercises.md(1 hunks)internal/exercises/catalog.yaml(2 hunks)internal/exercises/solutions/41_regex/regex.go(1 hunks)internal/exercises/templates/41_regex/regex.go(1 hunks)internal/exercises/templates/41_regex/regex_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/exercises.md
- internal/exercises/catalog.yaml
🔇 Additional comments (5)
internal/exercises/templates/41_regex/regex_test.go (1)
1-99: LGTM! Comprehensive test coverage.The test suite is well-structured with table-driven tests covering:
- Valid and invalid inputs for each function
- Edge cases (empty strings, boundary conditions)
- Case sensitivity where applicable
- Various malformed inputs
The test cases align well with the expected regex implementations and provide good coverage for the learning exercise.
internal/exercises/templates/41_regex/regex.go (1)
1-37: LGTM! Clear template structure for learners.The template provides:
- Clear task descriptions with numbered objectives
- Helpful TODO comments referencing specific regexp methods
- Appropriate zero-value returns for type safety
- Well-documented expected behavior for each function
This will guide learners effectively through implementing the regex patterns.
internal/exercises/solutions/41_regex/regex.go (3)
5-10: LGTM! Email validation pattern is appropriate.The regex pattern correctly validates basic email format and passes all test cases. The simplified pattern is suitable for this educational exercise.
23-28: LGTM! Vowel replacement is correct.The pattern handles both uppercase and lowercase vowels correctly using a character class. Simple and effective implementation.
30-35: LGTM! Phone number validation is precise.The pattern correctly enforces the US phone format (XXX) XXX-XXXX with proper anchoring and escaped special characters. All test cases pass correctly.
@zhravan resolved |
|
@zhravan Could you please review this PR when you get a chance and add the relevant tags? |
|
@zhravan Just a friendly reminder about this PR. If everything looks good, kindly review and merge it whenever you get a chance. Thanks! |
2 participants
Summary
Adds a new regular expressions exercise (28_regex) to the GoLearn exercise catalog. This exercise teaches students how to use Go's
regexppackage for pattern matching, text processing, and validation tasks.Motivation: Regular expressions are a fundamental skill for text processing and validation in Go. This exercise fills a gap in the curriculum by providing hands-on practice with common regex patterns and use cases.
Checklist
make verifyorgolearn verify <slug>Screenshots / Output (if CLI UX)
Before:
After:
Related issues
Fixes #80
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.