Skip to content

Reverse-string: add a test case with a longer string #1983

Open

Description

@norbs57

I recently mentored a student on ReverseString who had started off with a naive algorithm that had quadratic complexity. It was a bit disappointing to see that in the benchmarks, the speedup was not that great. I noticed that all the sample strings in the test suite are rather short. Maybe we could add at least one longer string so that the performance gains of a better algorithm become more impressive?

Activity

  1. SaschaMann commented on Mar 8, 2022

    @SaschaMann
    Contributor

    Do you know how long the string would have to be for it to be noticeable in benchmarks?

    Usually that would be something that an analyzer or mentor would discuss with the student though, not part of the test suite.

  2. norbs57 commented on Mar 8, 2022

    @norbs57
    ContributorAuthor

    With the current test suite (in Go), the speedup is about factor 2. After adding a 100 character string to the test, the speedup is closer to a factor 4. (Usual disclaimers about micro-benchmarks apply.)

  3. norbs57 commented on Mar 8, 2022

    @norbs57
    ContributorAuthor

    To be more precise - the Go benchmark test for ReverseString currently takes the average of the execution time of all test cases defined in canonical-data.json. If the benchmark data consist of a single 100-character string only, then the speedup is about a factor 5.

  4. SaschaMann commented on Apr 16, 2022

    @SaschaMann
    Contributor

    I think a 100 character string would definitely be reasonable to have. I'd be in favour of adding it if you want to open a PR.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions