Skip to content

refactor: fmt.Printf in strings/kmp #585

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

Closed

Conversation

uh-zz
Copy link
Contributor

@uh-zz uh-zz commented Oct 21, 2022

Description

Hi!
This is my first pull request to this repository!

Refactored fmt.Printf so that it is done in each block, and do not insert a newline code at the beginning of fmt.Printf.

I'll also include the output below.

go test -v
=== RUN   TestKMP
=== RUN   TestKMP/String_comparison_on_single_pattern_match
        comparing characters C a at positions 0 0
        comparing characters P a at positions 1 0
        comparing characters M a at positions 2 0
        comparing characters _ a at positions 3 0
        comparing characters a a at positions 4 0 - match
        comparing characters n n at positions 5 1 - match
        comparing characters n n at positions 6 2 - match
        comparing characters u o at positions 7 3
        comparing characters u a at positions 7 0
        comparing characters a a at positions 8 0 - match
        comparing characters l n at positions 9 1
        comparing characters l a at positions 9 0
        comparing characters _ a at positions 10 0
        comparing characters c a at positions 11 0
        comparing characters o a at positions 12 0
        comparing characters n a at positions 13 0
        comparing characters f a at positions 14 0
        comparing characters e a at positions 15 0
        comparing characters r a at positions 16 0
        comparing characters e a at positions 17 0
        comparing characters n a at positions 18 0
        comparing characters c a at positions 19 0
        comparing characters e a at positions 20 0
        comparing characters _ a at positions 21 0
        comparing characters a a at positions 22 0 - match
        comparing characters n n at positions 23 1 - match
        comparing characters n n at positions 24 2 - match
        comparing characters o o at positions 25 3 - match
        comparing characters u u at positions 26 4 - match
        comparing characters n n at positions 27 5 - match
        comparing characters c c at positions 28 6 - match
        comparing characters e e at positions 29 7 - match
=== RUN   TestKMP/String_comparison_on_multiple_pattern_match
        comparing characters A A at positions 0 0 - match
        comparing characters A A at positions 1 1 - match
        comparing characters B B at positions 2 2 - match
        comparing characters A A at positions 3 3 - match
=== RUN   TestKMP/String_comparison_with_not_found_pattern
        comparing characters A A at positions 0 0 - match
        comparing characters A A at positions 1 1 - match
        comparing characters B B at positions 2 2 - match
        comparing characters A C at positions 3 3
        comparing characters A A at positions 3 0 - match
        comparing characters A A at positions 4 1 - match
        comparing characters C B at positions 5 2
        comparing characters C A at positions 5 1
        comparing characters C A at positions 5 0
        comparing characters A A at positions 6 0 - match
        comparing characters A A at positions 7 1 - match
        comparing characters D B at positions 8 2
        comparing characters D A at positions 8 1
        comparing characters D A at positions 8 0
        comparing characters A A at positions 9 0 - match
        comparing characters A A at positions 10 1 - match
        comparing characters B B at positions 11 2 - match
        comparing characters A C at positions 12 3
        comparing characters A A at positions 12 0 - match
        comparing characters A A at positions 13 1 - match
        comparing characters B B at positions 14 2 - match
        comparing characters A C at positions 15 3
        comparing characters A A at positions 15 0 - match
--- PASS: TestKMP (0.00s)
    --- PASS: TestKMP/String_comparison_on_single_pattern_match (0.00s)
    --- PASS: TestKMP/String_comparison_on_multiple_pattern_match (0.00s)
    --- PASS: TestKMP/String_comparison_with_not_found_pattern (0.00s)

Checklist

  • Added description of change
  • Added tests and example, test must pass
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

@tjgurwara99
Copy link
Member

tjgurwara99 commented Oct 22, 2022

The current implementation is not quite up to standards, which is one of the reasons its not been improved in quite a bit. The current implementation prints to the standard output - not something that we want to do and also the original KMP algorithm returns an array of all positions where a match for the word occur and the length of this array. I believe if you return a slice of the all positions where it matches instead of returning the first occurrence, it would be better. Otherwise, I think this PR is not useful since all your changes would be removed anyways after we get to improving it.

@uh-zz
Copy link
Contributor Author

uh-zz commented Oct 22, 2022

@tjgurwara99
Thanks for the review!
I understand that this pull request is no longer needed due to the current non-standard implementation.
If there is not already an improved implementation in other pull requests, I will create an improved pull request.

@raklaptudirm
Copy link
Member

@uh-zz there is no effort currently to improve this algorithm. A pr from your side would be welcome.

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Nov 22, 2022
@github-actions
Copy link

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants