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

EndsWith should use rfind() #573

Closed
lichray opened this issue Jan 20, 2016 · 2 comments
Closed

EndsWith should use rfind() #573

lichray opened this issue Jan 20, 2016 · 2 comments

Comments

@lichray
Copy link
Contributor

lichray commented Jan 20, 2016

https://github.com/philsquared/Catch/blob/7424b23bfb4423fd1213141633844f09faaff8bc/single_include/catch.hpp#L1089

.find() is used.

@philsquared
Copy link
Collaborator

Wow that's terrible code! Did I write that? (checks... yes I did. I am ashamed).
You are quite right that, given the approach being used it should be using rfind().
But the approach itself seems like an awkward way to do it. I can only imagine that I started with Contains() and made the minimum changes necessary to morph it into StartsWith(), then EndsWith().
But given that I already had the functions startsWith() and endsWith() (https://github.com/philsquared/Catch/blob/master/include/internal/catch_common.hpp#L19) I don't understand why I didn't use them.

So, thanks for bringing this up! I'll take a closer look later and clean things up a bit (fixing the latent bug in the process).

philsquared added a commit that referenced this issue Jan 22, 2016
philsquared added a commit that referenced this issue Jun 7, 2016
@horenmar
Copy link
Member

horenmar commented Jan 8, 2017

This has been since fixed, albeit it would probably be better if every endsWith and startsWith calls didn't allocate a brand new string.

@horenmar horenmar closed this as completed Jan 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants