Skip to content

Conversation

@WazedKhan
Copy link
Owner

@WazedKhan WazedKhan commented Nov 5, 2025

Summary by CodeRabbit

  • New Features

    • Added implementations for "Longest Substring Without Repeating Characters" in Go and Python.
    • Added several additional array/string algorithm solutions (duplicate check, anagram check, two-sum).
  • Tests

    • Added unit tests validating the new longest-substring implementations across multiple cases.
  • Refactor

    • Simplified return logic in an existing parentheses-validation function for improved clarity.

@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Go: core + tests
Go/leetcode.go, Go/leetcode_test.go
Appended LengthOfLongestSubstring(s string) int using sliding-window with last-index map; simplified IsValidParentheses return to return len(stack) == 0; added Test_LengthOfLongestSubstring with multiple cases.
Python: solution
LeetCode/medium/longest_substring_without_repeat.py
New Solution class with lengthOfLongestSubstring(self, s: str) -> int implementing sliding-window and dictionary last-index tracking.
Python: tests
tests/test_leetcode_medium.py
New pytest module with parameterized tests for lengthOfLongestSubstring; placeholders for other problems included.
Weekly roundtable: array hashing
weekly_roundtable/array_hasing/nov_09_2025.py
New Solution methods: hasDuplicate, isAnagram, and twoSum (notes: truthiness usage and return type mismatch may need attention).

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas to focus review on:
    • Correctness of sliding-window index updates and off-by-one handling in both Go and Python implementations.
    • Test coverage completeness and edge cases (empty string, all-unique, all-duplicate, repeated patterns).
    • weekly_roundtable/array_hasing/nov_09_2025.py: truthiness check in hasDuplicate (0 handling) and twoSum return type consistency.

Possibly related PRs

Poem

🐰
I hop through bytes and letters bright,
A sliding window finds what's right,
Maps and indices, nimble paws,
Tests applaud my clever cause,
Longest substring — carrot-sized delight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 'Longest Substring Without Repeating Characters' directly matches the primary feature added across multiple language implementations (Go, Python) and tests throughout the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 03-longest-substring-without-repeating-characters

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

@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: 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 ever 0, 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] = 1

Alternatively, 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 Counter from 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 []
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40da677 and ec70bfc.

📒 Files selected for processing (1)
  • weekly_roundtable/array_hasing/nov_09_2025.py (1 hunks)


for idx in range(len(s)):
s_char = s[idx]
t_chat = t[idx]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +32 to +40
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]]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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].

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.

2 participants