Skip to content

Conversation

oulaindex
Copy link

Description

This pull request updates the implementation of the Manacher algorithm for finding the longest palindromic substring.
Summary of changes:

  • The old implementation in manacher.rs contains a critical logic bug:
    • The variables
      let mut current_center: usize = 0;
      let mut right_from_current_center: usize = 0;
      are only updated within the block guarded by
      if right_from_current_center > i && i > current_center,
      which is never true when both variables are zero at initialization.
      As a result, the logic for updating current_center and right_from_current_center can never execute inside the conditional block at the start, causing these variables to remain unchanged at zero during the initial iterations. Although the final result remains correct due to redundant expansions, this leads to unnecessary repeated checks and significantly reduces the efficiency of the algorithm.
  • The new implementation (manacher.rs) fixes this by:
    • Ensuring the logic for updating the center and right boundary always functions correctly, using the standard and widely accepted method for Manacher’s algorithm.
    • Initializing and updating these variables in a manner consistent with canonical implementations and literature, ensuring correct and complete palindrome detection.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I ran bellow commands using the latest version of rust nightly.
  • I ran cargo clippy --all -- -D warnings just before my last commit and fixed any issue that was found.
  • I ran cargo fmt just before my last commit.
  • I ran cargo test just before my last commit and all tests passed.

Note:
This PR not only clarifies and streamlines the code, but, most importantly, it fixes a functional bug in the center and boundary assignment logic.
All tests pass, and the implementation follows the best practices and guidelines for Manacher’s algorithm.

@oulaindex oulaindex requested a review from imp2002 as a code owner September 9, 2025 16:33
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.36%. Comparing base (f2a23e9) to head (2f4c748).

Files with missing lines Patch % Lines
src/string/manacher.rs 95.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #911      +/-   ##
==========================================
+ Coverage   95.32%   95.36%   +0.04%     
==========================================
  Files         319      319              
  Lines       20807    20802       -5     
==========================================
+ Hits        19834    19838       +4     
+ Misses        973      964       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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