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

Implemented Str.Count() method #2342

Merged
merged 3 commits into from
Sep 27, 2023
Merged

Conversation

SudhanshuJoshi09
Copy link
Contributor

  1. Implemented Str.Count() method
  2. Included tests for validating str.count.

@SudhanshuJoshi09 SudhanshuJoshi09 marked this pull request as draft September 27, 2023 09:54
1. Added count function for strings str.count().
1. Added a test for str.count() and included it over CMakeList.txt
@SudhanshuJoshi09 SudhanshuJoshi09 marked this pull request as ready for review September 27, 2023 12:23
Copy link
Collaborator

@Thirumalai-Shaktivel Thirumalai-Shaktivel left a comment

Choose a reason for hiding this comment

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

This could have been implemented in IntrinsicFunction.
But this is fine for now, I think.

Looks good, thank you!

Copy link
Collaborator

@anutosh491 anutosh491 left a comment

Choose a reason for hiding this comment

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

looks good !

@certik certik merged commit 03a9cf1 into lcompilers:main Sep 27, 2023
12 checks passed
@certik
Copy link
Contributor

certik commented Sep 27, 2023

Thanks!

@SudhanshuJoshi09 SudhanshuJoshi09 deleted the str-count branch September 28, 2023 07:06
@OculusMode
Copy link
Contributor

OculusMode commented Oct 2, 2023

Small nitpicking on this but why to use an algorithm having O(n * m), we do have algos like boyer-moore, or even rabin karp if we do not wish to eat space.

some ref for this: https://en.wikipedia.org/wiki/String-searching_algorithm

or if you want to check out more of what cpython does, this is a nice place:

/* fast search/count implementation, based on a mix between boyer-
moore and horspool, with a few more bells and whistles on the top.
for some more background, see:
https://web.archive.org/web/20201107074620/http://effbot.org/zone/stringlib.htm */

via: https://github.com/python/cpython/blob/main/Objects/stringlib/fastsearch.h

@certik
Copy link
Contributor

certik commented Oct 2, 2023

@OculusMode awesome, thanks! Can you post your comment as a new issue?

Then we can send PRs to implement it. If you are interested in this, that would be great.

@OculusMode
Copy link
Contributor

Sure, can do that.

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

Successfully merging this pull request may close these issues.

5 participants