Skip to content

refactor(urlparser): use map lookup and factory functions#15

Merged
eannchen merged 1 commit intomainfrom
refactor/urlparser-map-lookup-factory
Jan 1, 2026
Merged

refactor(urlparser): use map lookup and factory functions#15
eannchen merged 1 commit intomainfrom
refactor/urlparser-map-lookup-factory

Conversation

@eannchen
Copy link
Copy Markdown
Owner

@eannchen eannchen commented Jan 1, 2026

Description

Improve urlparser package with two optimizations:

  1. Replace slice iteration with map lookup for O(1) host matching
  2. Add factory functions to eliminate duplicate HackerRank parser configs

Also expanded test coverage (13 → 27 cases) including error type assertions,
whitespace handling, and host variations.

Type of Change

  • New feature
  • Bug fix
  • Refactor
  • Documentation update
  • Build/CI-related change
  • Other (specify):

Checklist

  • My code follows the project's style.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • All new and existing unit tests pass locally with my changes.
  • I have manually tested the new functionality in the CLI.

Summary by CodeRabbit

Release Notes

  • Refactor

    • Optimized URL parsing logic for improved performance and maintainability.
  • Tests

    • Enhanced test coverage with additional validation for edge cases, error handling, and host variations.

✏️ Tip: You can customize this high-level summary in your review settings.

- Added new test cases for various LeetCode and HackerRank URL formats, including handling of query parameters, fragments, leading/trailing whitespace, and HTTP schemes.
- Introduced error handling for unsupported platforms and invalid URL formats in the parsing logic.
- Refactored platform-specific URL parsers to improve maintainability and support for different host variations.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 1, 2026

📝 Walkthrough

Walkthrough

Refactors the URL parser's platform configuration from a slice-based approach to a map-driven architecture with O(1) host lookups. Updates the platformParser struct to use a platform field instead of host, modifies Parse and NormalizeURL functions accordingly, and adds comprehensive test coverage for edge cases and error handling.

Changes

Cohort / File(s) Summary
Core Parser Refactoring
internal/urlparser/urlparser.go
Replaced slice-based platformParsers with map[string]platformParser for O(1) lookups. Introduced newLeetCodeParser and newHackerRankParser constructors. Removed host field from platformParser struct; added platform field of type core.Platform. Updated Parse function to perform direct map lookup by parsedURL.Host. Modified NormalizeURL to accept and use host parameter instead of hard-coded value. Updated error handling logic for unsupported platforms and invalid formats. Removed legacy hostToPlatform mapping and iteration patterns.
Test Expansion
internal/urlparser/urlparser_test.go
Added test cases covering query parameters, fragments, leading/trailing whitespace, numeric slugs, and HTTP/HTTPS scheme normalization for LeetCode and HackerRank. Introduced TestParse_ErrorTypes to validate specific error values (ErrUnsupportedPlatform, ErrInvalidProblemURLFormat). Introduced TestParse_HostVariations to verify behavior with uppercase, mixed-case, and www-prefixed hosts. Updated imports to include errors and errs packages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • feat: Add HackerRank support #7: Directly modifies the same urlparser components (platformParser struct, platformParsers initialization, Parse/NormalizeURL logic) and error types to add multi-platform (LeetCode/HackerRank) parsing.

Poem

🐰 A map hops faster than a slice, they say,
O(1) lookups brighten the parser's day!
From hard-coded hosts to a platform's embrace,
This refactor moves parsing to a better place. 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactoring: replacing slice iteration with map lookup and introducing factory functions for parser construction.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
internal/urlparser/urlparser_test.go (1)

288-322: Good documentation of host matching behavior.

These tests explicitly document that host matching is case-sensitive and that www.leetcode.com is not supported. While this matches the current map-based implementation, it may be unexpected for users since DNS hostnames are case-insensitive. Consider adding case normalization (e.g., strings.ToLower(parsedURL.Host)) in the Parse function to improve user experience.

🔎 Optional: Add case normalization for better UX

In internal/urlparser/urlparser.go, normalize the host to lowercase before lookup:

// O(1) lookup by host
parser, found := platformParsers[strings.ToLower(parsedURL.Host)]

Then update the map keys to lowercase and update these test cases to expect success instead of failure for uppercase/mixed-case hosts.

internal/urlparser/urlparser.go (1)

45-50: LGTM! Efficient map-based lookup.

The refactor from slice to map enables O(1) host lookup, a clear performance improvement. The factory functions eliminate configuration duplication for HackerRank's multiple hosts. Note that www.leetcode.com is intentionally not supported (only www.hackerrank.com is), which may surprise some users.

💡 Optional: Consider adding www.leetcode.com support

For consistency and better user experience, consider adding:

 var platformParsers = map[string]platformParser{
 	"leetcode.com":       newLeetCodeParser("leetcode.com"),
+	"www.leetcode.com":   newLeetCodeParser("www.leetcode.com"),
 	"hackerrank.com":     newHackerRankParser("hackerrank.com"),
 	"www.hackerrank.com": newHackerRankParser("www.hackerrank.com"),
 }

This would handle URLs copied from browsers that include the www subdomain.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d98785 and 4b4f463.

📒 Files selected for processing (2)
  • internal/urlparser/urlparser.go
  • internal/urlparser/urlparser_test.go
🧰 Additional context used
🧬 Code graph analysis (2)
internal/urlparser/urlparser_test.go (2)
internal/errs/errors.go (2)
  • ErrUnsupportedPlatform (22-22)
  • ErrInvalidProblemURLFormat (23-23)
internal/urlparser/urlparser.go (1)
  • Parse (58-88)
internal/urlparser/urlparser.go (2)
core/model.go (3)
  • Platform (37-37)
  • PlatformLeetCode (40-40)
  • ParsedURL (55-59)
internal/errs/errors.go (2)
  • ErrUnsupportedPlatform (22-22)
  • ErrInvalidProblemURLFormat (23-23)
🔇 Additional comments (8)
internal/urlparser/urlparser_test.go (4)

4-8: LGTM!

The new imports support error type validation using errors.Is() in the test cases, which is the idiomatic approach for sentinel error checking in Go.


47-88: LGTM! Excellent test coverage expansion.

These new test cases thoroughly cover edge cases including query parameters, fragments, whitespace handling, numeric slugs, and HTTP-to-HTTPS normalization. The tests validate that all variations correctly normalize to the canonical HTTPS format.


157-183: LGTM! Comprehensive HackerRank coverage.

The expanded test cases mirror the LeetCode improvements, ensuring consistent handling of query parameters, fragments, whitespace, and scheme normalization across both platforms.


241-286: LGTM! Proper error type validation.

This test function correctly uses errors.Is() to validate that the parser returns the appropriate sentinel errors (ErrUnsupportedPlatform and ErrInvalidProblemURLFormat) for different failure scenarios, ensuring reliable error handling.

internal/urlparser/urlparser.go (4)

14-19: LGTM! Clean struct refactor.

Replacing the host field with platform directly stores the platform type, eliminating the need for a separate mapping and making the code more straightforward.


21-31: LGTM! Well-structured factory function.

The factory function encapsulates LeetCode parser configuration and accepts a host parameter, enabling reuse for host variations while maintaining clean separation of concerns. The hardcoded HTTPS scheme ensures consistent normalization.


33-43: LGTM! Consistent factory pattern.

The HackerRank parser factory mirrors the LeetCode implementation, properly encapsulating platform-specific differences (path patterns, URL format) while eliminating duplication between hackerrank.com and www.hackerrank.com.


58-88: LGTM! Clean and efficient implementation.

The Parse function is well-structured with clear error handling:

  • Line 60: Properly trims whitespace from input
  • Lines 65-69: O(1) map lookup with path prefix validation
  • Lines 72-80: Robust slug extraction and validation
  • Lines 83-87: Clean result construction using the parser's platform field

The refactor successfully replaces iteration with direct map access, improving both performance and code clarity.

@eannchen eannchen merged commit bf6eaa8 into main Jan 1, 2026
8 checks passed
@eannchen eannchen deleted the refactor/urlparser-map-lookup-factory branch January 1, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant