Skip to content

Manacher's Algorithm#11

Open
dlesnoff wants to merge 14 commits into
TheAlgorithms:mainfrom
dlesnoff:manacher
Open

Manacher's Algorithm#11
dlesnoff wants to merge 14 commits into
TheAlgorithms:mainfrom
dlesnoff:manacher

Conversation

@dlesnoff

Copy link
Copy Markdown
Collaborator

Manacher's Algorithm finds the longest palindromic factor (substring) in linear time in the length of the string input.

@Panquesito7 Panquesito7 added the enhancement New feature or request label Apr 26, 2023

@SatinWukerORIG SatinWukerORIG left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! I like your raising error operations. Very clear comments, but just a few suggestions.

Comment thread strings/manacher.nim Outdated
Comment thread strings/manacher.nim Outdated
Comment thread strings/manacher.nim Outdated
Comment thread strings/manacher.nim
Comment thread strings/manacher.nim
Comment thread strings/manacher.nim
Comment thread strings/manacher.nim
Comment thread strings/manacher.nim Outdated
@dlesnoff dlesnoff requested a review from SatinWukerORIG July 7, 2023 08:21
@dlesnoff

dlesnoff commented Jul 7, 2023

Copy link
Copy Markdown
Collaborator Author

@SatinWuker Is it working on your side?

@SatinWukerORIG

Copy link
Copy Markdown
Member

@SatinWuker Is it working on your side?

It works really well 👍👍👍 thank you for your clear explanation and helpful comment on the file haha!

@dlesnoff

dlesnoff commented Jul 9, 2023

Copy link
Copy Markdown
Collaborator Author

As one may notice, I compute the indexes whenever I ask for the length and the string.
This might impact performance for very long strings. We may prefer to pass the indexes as parameters and determine the length or the string itself from this precomputed argument.
A better way to handle this would be to compute an object with None/Option[T] attributes, that we fill with precisely the information we want.

@dlesnoff

dlesnoff commented Jul 9, 2023

Copy link
Copy Markdown
Collaborator Author

@SatinWuker

It works really well +1+1+1 thank you for your clear explanation and helpful comment on the file haha!

Do you still require the variable name changes or would you like to approve the pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants