Fix Boyer-Moore bad character shift having no effect - #15053
Open
satyamkumar-builds wants to merge 1 commit into
Open
Fix Boyer-Moore bad character shift having no effect#15053satyamkumar-builds wants to merge 1 commit into
satyamkumar-builds wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your change:
In
strings/boyer_moore_search.py,bad_character_heuristic()reassigned thefor-loop variableiinside the loop body: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
whileloop so the shift actually applies, usingi = max(i + 1, mismatch_index - match_index)to guarantee forward progress (themaxalso covers the case where the mismatched character is absent from the pattern, wherematch_index == -1makes the raw difference negative or a no-op shift).Verification:
text='ABCDEFGHIJKLMNOP...',pattern='MNOP') now takes 9 iterations instead of 29.Fixes #14844