-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
feat: added z_algorithm in strings #1581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@Panquesito7 Kindly look into my PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please look at the contribution guidelines
Hi @ayaankhan98 Can you be more specific where I made mistake. I am new to PR's. |
Updated z_function.cpp as per contribution guidelines. Fixed Link using github markdown syntax Created a separate function for tests and covered the corner case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 😄👍
More comments added to the code Co-authored-by: David Leal <[email protected]>
Some more documentation added as per contribution guidelines. Co-authored-by: David Leal <[email protected]>
comments added Co-authored-by: David Leal <[email protected]>
Thank you :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please enable GitHub Actions in your repository of this fork in this link: https://github.com/RitikaGupta8734/C-Plus-Plus/actions
strings/z_function.cpp
Outdated
* \returns a vector of starting indexes where pattern is found in the text | ||
*/ | ||
std::vector<int> find_pat_in_text(const std::string &pattern, const std::string &text) { | ||
int text_length = text.size(), pattern_length = pattern.size(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using uint64_t
for non-negative values (or their appropriate size: uint32_t
, uint16_t
, uint8_t
) or int64_t
for negative values. Check other parts of the code (reference). 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the required changes. Kindly review :)
Co-authored-by: David Leal <[email protected]>
0c7515e
to
d82a96f
Compare
d82a96f
to
edff958
Compare
Updated int -> uint64_t for non-negative values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Almost there. 😄
strings/z_function.cpp
Outdated
std::string text1 = "alskfjaldsabc1abc1abcbksbcdnsdabcabc"; | ||
std::string pattern1 = "abc"; | ||
|
||
std::vector<uint64_t> matching_indexes1 = find_pat_in_text(pattern1, text1); | ||
assert((matching_indexes1 == std::vector<uint64_t>{10, 14, 18, 30, 33})); | ||
|
||
// corner case | ||
std::string text2 = "greengrass"; | ||
std::string pattern2 = "abc"; | ||
|
||
std::vector<uint64_t> matching_indexes2 = find_pat_in_text(pattern2, text2); | ||
assert((matching_indexes2 == std::vector<uint64_t>{})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation of what this code is doing here. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments for the same.
Co-authored-by: David Leal <[email protected]>
Co-authored-by: David Leal <[email protected]>
c9919d6
to
0737aff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there! It's just to accept these suggestions, and you're done! 😄🎉
Co-authored-by: David Leal <[email protected]>
Co-authored-by: David Leal <[email protected]>
88796b2
to
8ee1415
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, @RitikaGupta8734! It's been awesome for your first contribution here! We hope you keep contributing! 😄👍🎉
Thanks a lot @Panquesito7 :) |
Hi @ayaankhan98 kindly review the PR again and let me know the feedback if any. |
Hi @Panquesito7 will my PR be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Thank You all ! :) |
Description of Change
Added Z-Algorithm for pattern matching in strings.
Checklist
Notes:
Kindly review my PR and let me know any corrections.