Skip to content
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

feat: added z_algorithm in strings #1581

Merged
merged 20 commits into from
Sep 4, 2021

Conversation

RitikaGupta8734
Copy link
Contributor

@RitikaGupta8734 RitikaGupta8734 commented Aug 30, 2021

Description of Change

Added Z-Algorithm for pattern matching in strings.

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • PR title follows semantic commit guidelines
  • I acknowledge that all my contributions will be made under the project's license.

Notes:
Kindly review my PR and let me know any corrections.

@RitikaGupta8734
Copy link
Contributor Author

@Panquesito7 Kindly look into my PR.

Copy link
Member

@ayaankhan98 ayaankhan98 left a 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

@RitikaGupta8734
Copy link
Contributor Author

RitikaGupta8734 commented Aug 31, 2021

Hi @ayaankhan98 Can you be more specific where I made mistake. I am new to PR's.

strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
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
@Panquesito7 Panquesito7 added enhancement New feature or request requested changes changes required labels Sep 1, 2021
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Great work! 😄👍

strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
RitikaGupta8734 and others added 3 commits September 1, 2021 09:48
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]>
@RitikaGupta8734
Copy link
Contributor Author

Great work!

Thank you :)

Copy link
Member

@Panquesito7 Panquesito7 left a 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

* \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();
Copy link
Member

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). 🙂

Copy link
Contributor Author

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 :)

strings/z_function.cpp Outdated Show resolved Hide resolved
RitikaGupta8734 and others added 2 commits September 2, 2021 01:10
Updated int -> uint64_t for non-negative values
Copy link
Member

@Panquesito7 Panquesito7 left a 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 Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
Comment on lines 74 to 85
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>{}));
Copy link
Member

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. 🙂

Copy link
Contributor Author

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.

Copy link
Member

@Panquesito7 Panquesito7 left a 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! 😄🎉

strings/z_function.cpp Outdated Show resolved Hide resolved
strings/z_function.cpp Show resolved Hide resolved
strings/z_function.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Panquesito7 Panquesito7 left a 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! 😄👍🎉

@RitikaGupta8734
Copy link
Contributor Author

Awesome work, @RitikaGupta8734! It's been awesome for your first contribution here! We hope you keep contributing!

Thanks a lot @Panquesito7 :)

@Panquesito7 Panquesito7 added approved Approved; waiting for merge and removed requested changes changes required labels Sep 3, 2021
@RitikaGupta8734
Copy link
Contributor Author

Hi @ayaankhan98 kindly review the PR again and let me know the feedback if any.

@RitikaGupta8734
Copy link
Contributor Author

Hi @Panquesito7 will my PR be merged?

Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ayaankhan98 ayaankhan98 merged commit b82e2cd into TheAlgorithms:master Sep 4, 2021
@RitikaGupta8734
Copy link
Contributor Author

Thank You all ! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants