-
Notifications
You must be signed in to change notification settings - Fork 1
03: Longest Substring Without Repeating Characters #110
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: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds LengthOfLongestSubstring implementations in Go and Python with sliding-window and last-index maps, corresponding tests in Go and pytest for Python, and simplifies IsValidParentheses return logic in Go. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (test / main)
participant Algo as SlidingWindowAlgo
participant Map as CharIndexMap
participant Window as [start,end] Tracker
Caller->>Algo: call LengthOfLongestSubstring(s)
Algo->>Map: init empty map
Algo->>Window: start = 0
loop for each index i, char c in s
Algo->>Map: lookup c
alt found and map[c] >= start
Algo->>Window: start = map[c] + 1
Note right of Algo `#F6E8D7`: shrink window (changed interaction)
end
Algo->>Map: map[c] = i
Algo->>Window: update max_len = max(max_len, i - start + 1)
end
Algo->>Caller: return max_len
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (3)
weekly_roundtable/array_hasing/nov_09_2025.py (3)
8-12: Check for None explicitly rather than relying on truthiness.The truthiness check on line 10 works in this case because you store
1, but it's fragile and confusing. If the stored value were ever0,False, or an empty collection, the check would fail incorrectly.Apply this diff to make the check explicit and clearer:
for val in nums: - gg = seen.get(val) - if gg: + if val in seen: return True seen[val] = 1Alternatively, simplify using a set:
- def hasDuplicate(self, nums: List[int]) -> bool: - seen = {} # {key: valu} - - for val in nums: - gg = seen.get(val) - if gg: - return True - seen[val] = 1 - - return False + def hasDuplicate(self, nums: List[int]) -> bool: + seen = set() + for val in nums: + if val in seen: + return True + seen.add(val) + return False
23-30: Consider using Counter for more idiomatic code.The manual frequency counting works but Python's
Counterfrom collections is more concise and idiomatic.+from collections import Counter + def isAnagram(self, s: str, t: str) -> bool: if len(s) != len(t): return False - - s_map = {} - t_map = {} - - for idx in range(len(s)): - s_char = s[idx] - t_char = t[idx] - - s_map[s_char] = s_map.get(s_char, 0) + 1 - t_map[t_char] = t_map.get(t_char, 0) + 1 - - return s_map == t_map + return Counter(s) == Counter(t)
34-40: Consider single-pass optimization.The current two-pass approach works, but you can optimize it to a single pass by checking for the complement while building the map.
def twoSum(self, nums: List[int], target: int) -> List[int]: hash_map = {} for idx in range(len(nums)): - hash_map[nums[idx]] = idx - - for idx in range(len(nums)): diff = target - nums[idx] - if diff in hash_map and hash_map[diff] != idx: + if diff in hash_map: return [idx, hash_map[diff]] + hash_map[nums[idx]] = idx + return []
|
|
||
| for idx in range(len(s)): | ||
| s_char = s[idx] | ||
| t_chat = t[idx] |
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 typo in variable name.
t_chat should be t_char for consistency and clarity.
- t_chat = t[idx]
+ t_char = t[idx]
s_map[s_char] = s_map.get(s_char, 0) + 1
- t_map[t_chat] = t_map.get(t_chat, 0) + 1
+ t_map[t_char] = t_map.get(t_char, 0) + 1🤖 Prompt for AI Agents
In weekly_roundtable/array_hasing/nov_09_2025.py around line 25, there is a typo
where the variable is named t_chat; rename it to t_char to match the intended
name and code style, update all occurrences/references in the surrounding scope
(assignments, uses, and any function parameters) to the new t_char name to avoid
NameError and keep consistency.
| def twoSum(self, nums: List[int], target: int) -> List[int]: | ||
| hash_map = {} | ||
| for idx in range(len(nums)): | ||
| hash_map[nums[idx]] = idx | ||
|
|
||
| for idx in range(len(nums)): | ||
| diff = target - nums[idx] | ||
| if diff in hash_map and hash_map[diff] != idx: | ||
| return [idx, hash_map[diff]] |
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.
Add explicit return for the no-pair case.
The function can implicitly return None when no pair sums to the target, which violates the List[int] return type annotation.
def twoSum(self, nums: List[int], target: int) -> List[int]:
hash_map = {}
for idx in range(len(nums)):
hash_map[nums[idx]] = idx
for idx in range(len(nums)):
diff = target - nums[idx]
if diff in hash_map and hash_map[diff] != idx:
return [idx, hash_map[diff]]
+ return [] # or raise an exception if input guarantees a solution exists📝 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.
| def twoSum(self, nums: List[int], target: int) -> List[int]: | |
| hash_map = {} | |
| for idx in range(len(nums)): | |
| hash_map[nums[idx]] = idx | |
| for idx in range(len(nums)): | |
| diff = target - nums[idx] | |
| if diff in hash_map and hash_map[diff] != idx: | |
| return [idx, hash_map[diff]] | |
| def twoSum(self, nums: List[int], target: int) -> List[int]: | |
| hash_map = {} | |
| for idx in range(len(nums)): | |
| hash_map[nums[idx]] = idx | |
| for idx in range(len(nums)): | |
| diff = target - nums[idx] | |
| if diff in hash_map and hash_map[diff] != idx: | |
| return [idx, hash_map[diff]] | |
| return [] # or raise an exception if input guarantees a solution exists |
🤖 Prompt for AI Agents
weekly_roundtable/array_hasing/nov_09_2025.py around lines 32-40: the twoSum
method can implicitly return None if no pair is found, violating the List[int]
annotation; add an explicit return path by raising a clear exception (e.g.,
raise ValueError("No two sum solution")) or returning an explicit empty list at
the end of the function so the return type is always List[int].
Summary by CodeRabbit
New Features
Tests
Refactor