Skip to content

added mock interview problem - #477

Open
krishnadheerajkrovi wants to merge 1 commit into
super30admin:masterfrom
krishnadheerajkrovi:master
Open

added mock interview problem#477
krishnadheerajkrovi wants to merge 1 commit into
super30admin:masterfrom
krishnadheerajkrovi:master

Conversation

@krishnadheerajkrovi

@krishnadheerajkrovi krishnadheerajkrovi commented Jul 16, 2026

Copy link
Copy Markdown

Please review

@super30admin

Copy link
Copy Markdown
Owner

The solution correctly identifies the peak of the mountain array using binary search, then performs two separate binary searches - one on the ascending part (with increasing target comparison) and one on the descending part (with decreasing target comparison). This is the standard approach for this problem.

Strengths:

  • Clean separation of concerns with helper methods for binary searches
  • Correctly handles the two-phase nature of the mountain array
  • Uses proper binary search boundaries

Areas for Improvement:

  1. Edge Case Handling: The solution doesn't handle edge cases well:

    • If peak == 0 (monotonically decreasing array), the descent search will have peak+1 > length-1, causing issues
    • If peak == length-1 (monotonically increasing array), the descent search will have peak+1 > length-1
    • Empty or single-element mountain arrays aren't handled
  2. Binary Search Logic: In binarySearch2, the comparison logic is inverted - when searching descending order, if mid_ele > target, you should move left (r = mid - 1), not right. Currently it does l = mid + 1 when mid_ele > target, which is incorrect for descending search.

  3. Peak Finding: The peak finding logic could be more robust. When l == r, the loop ends, but this should be verified to ensure peak is indeed the maximum element.

  4. Code Style: Missing docstrings for helper methods, and the code could benefit from more descriptive variable names.

  5. API Calls: The solution makes multiple calls to mountain_arr.get() which could be expensive in real scenarios. While not critical for correctness, caching values when possible could be an optimization.

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