Skip to content

binary search 2 done#2323

Open
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master
Open

binary search 2 done#2323
pranjay01 wants to merge 1 commit intosuper30admin:masterfrom
pranjay01:master

Conversation

@pranjay01
Copy link

No description provided.

@super30admin
Copy link
Owner

Your solution for Problem1.py is well done. You have correctly implemented the binary search to find the first and last occurrences of the target. The time and space complexity are optimal. Here are a few suggestions for improvement:

  1. In the first binary search, the condition to check for the first occurrence is correct, but note that the code breaks immediately after setting leftIndex. This is efficient. However, consider that the first binary search might not be entirely standard: you break as soon as you find the first occurrence. This is acceptable, but typically we might write it as a loop that continues until the condition is met. Your approach is fine.

  2. In the second binary search, you start from leftIndex (the first occurrence) to the end. This is a good optimization because the last occurrence must be at or after the first occurrence.

  3. The code could benefit from more comments to explain the logic, especially for someone who is reading it for the first time. For example, you could explain why you are breaking in the first binary search when you find the first occurrence.

  4. There is a very minor issue: in the first binary search, the condition if target == nums[mid] is followed by an inner condition. However, note that if the inner condition is not met (i.e., if it is not the first occurrence), you do not update the pointers. Instead, you rely on the else part of the outer condition? Actually, in your code, after the inner condition, you break if it is the first occurrence. But if it is not the first occurrence, you don't update the pointers. This is a problem.

Wait, let me look again: in the first binary search, when target == nums[mid], you check if it is the first occurrence. If it is, you set leftIndex and break. If it is not, you don't update the pointers? Actually, you don't have an else for the inner condition. This means that if the target is found but it is not the first occurrence, the code will fall through to the next conditions? But that is not correct.

This is a critical error. In the first binary search, when you find that nums[mid] == target but it is not the first occurrence (i.e., nums[mid-1] is also target), you should continue searching to the left. However, in your code, you do not handle this. Instead, you break out of the loop only when you find the first occurrence. But if you don't find the first occurrence at mid, you don't update the pointers and then the loop continues? Actually, no: after the inner condition, you break only if it is the first occurrence. If it is not, you don't break, but then the code continues to the next if statements. This is incorrect.

Let me simulate with an example: nums = [5,7,7,8,8,10], target=8.

  • Initially, left=0, right=5. mid=2 -> nums[2]=7 <8 -> left becomes 3.
  • Now left=3, right=5. mid=4 -> nums[4]=8 == target. Then check: mid=4, left=3 -> mid != left. Also, check nums[3]=8 which is equal to target? So it is not the first occurrence. But in your code, you do not update the pointers in this case. Instead, you just break out of the inner condition and then the code goes to the next if? Actually, after the inner condition, there is no else, so the code will proceed to the next if statements. But the next if is if target > nums[mid] which is false, and then else which is executed: right = mid-1? So for mid=4, we set right=3. Then we continue with left=3, right=3. Then mid=3 -> nums[3]=8 == target. Then check: mid=3 and left=3 -> mid==left? So it is the first occurrence? Actually, you check if mid == left or target != nums[mid-1]. For mid=3, left=3 -> mid==left is true, so you set leftIndex=3 and break. So it works in this case.

But why? Because when you find a target that is not the first occurrence, you effectively do nothing in the inner condition, and then the code falls through to the else clause (because target is not greater than nums[mid] and not less) so you set right = mid-1. This moves the search to the left. So it actually works.

However, this is not the most straightforward way. Typically, we would explicitly handle the case when we find the target but it is not the first occurrence by setting high = mid-1. Your code does this implicitly by falling through to the else. But note: the else clause is for when nums[mid] >= target? Actually, your code has:

  • if target == nums[mid]: ... [inner condition]
  • if target > nums[mid]: left = mid+1
  • else: right = mid-1

So when target == nums[mid], the inner condition is executed. If it is not the first occurrence, the inner condition does nothing, and then the code moves to the next if. Since target is not greater than nums[mid] (they are equal), it goes to the else and sets right=mid-1. This is correct because we want to search the left half for the first occurrence.

So the code is correct. However, it is a bit unconventional and might be confusing. It would be clearer to explicitly set high = mid-1 when the target is found but it is not the first occurrence. For example:

if target == nums[mid]:
if mid == left or nums[mid-1] != target:
leftIndex = mid
break
else:
right = mid - 1
elif target > nums[mid]:
left = mid+1
else:
right = mid-1

This makes the intention clear. Your current code relies on the else clause of the subsequent conditions to adjust the pointer, which is correct but less readable.

Similarly, in the second binary search, you have the same structure. It works, but it could be clearer.

Overall, the solution is correct and efficient. The code quality is good, but could be improved by making the logic more explicit.

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