Skip to content

Fix Boyer-Moore bad character shift having no effect - #15053

Open
satyamkumar-builds wants to merge 1 commit into
TheAlgorithms:masterfrom
satyamkumar-builds:fix/14844-boyer-moore-shift
Open

Fix Boyer-Moore bad character shift having no effect#15053
satyamkumar-builds wants to merge 1 commit into
TheAlgorithms:masterfrom
satyamkumar-builds:fix/14844-boyer-moore-shift

Conversation

@satyamkumar-builds

Copy link
Copy Markdown

Describe your change:

In strings/boyer_moore_search.py, bad_character_heuristic() reassigned the for-loop variable i inside the loop body:

for i in range(self.textLen - self.patLen + 1):
    ...
    i = (mismatch_index - match_index)  # no effect on iteration

In Python, reassigning the loop variable does not change the iteration, so the bad-character shift was dead code. The search checked every position sequentially — brute-force O(n·m) — while the module docstring advertises Boyer-Moore O(n/m).

Fix: convert the loop to a while loop so the shift actually applies, using i = max(i + 1, mismatch_index - match_index) to guarantee forward progress (the max also covers the case where the mismatched character is absent from the pattern, where match_index == -1 makes the raw difference negative or a no-op shift).

Verification:

  • All doctests pass.
  • 2000 randomized text/pattern comparisons against brute-force search: identical results.
  • The reproduction from the issue (text='ABCDEFGHIJKLMNOP...', pattern='MNOP') now takes 9 iterations instead of 29.

Fixes #14844

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Does your submission pass tests?

The bad_character_heuristic() reassigned the for-loop variable i
inside the loop body, which has no effect on iteration in Python.
As a result the bad-character shift was dead code and the search
degenerated into brute-force O(n*m) checking every position,
while still claiming O(n/m) in the module docstring.

Convert the loop to a while loop so the shift actually applies,
guaranteeing at least one position of progress per iteration via
max(i + 1, mismatch_index - match_index).

Verified: all doctests pass, 2000 randomized comparisons against
brute-force search pass, and the example from the issue now takes
9 iterations instead of 29.

Fixes TheAlgorithms#14844
@algorithms-keeper algorithms-keeper Bot added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed labels Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviews This PR is ready to be reviewed enhancement This PR modified some existing files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Boyer-Moore bad character shift has no effect (for-loop variable reassignment)

2 participants