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

Documentation of Data.Text.replace #303

Open
NorfairKing opened this issue Nov 2, 2020 · 1 comment
Open

Documentation of Data.Text.replace #303

NorfairKing opened this issue Nov 2, 2020 · 1 comment

Comments

@NorfairKing
Copy link

The docs say:

O(m+n)
[...]
In (unlikely) bad cases, this function's time complexity degrades towards O(n*m).

How unlikely are these bad cases? In any case the O(m+n) is misleading if the worst case is proportional to n*m.
I think we may want to spell this out further.

@kozross
Copy link

kozross commented Mar 15, 2021

These bad cases arise in similar situations to when BMH would do badly. The way replace (and indeed, all string matching provided by this library) works is essentially a BMH variant, designed to be zero-allocating, based on this here.

I would say that this should go beyond spelling out further - the choice of implementation is poor. I've exhibited this previously, and while the method I use can't port 1-for-1 (due to the UTF-16 encoding), it can probably be made considerably better than what we have right now. Furthermore, the current implementation of all string matching functions is needlessly partial, and should be fixed not to be. I've already done this in text-ascii, without any loss of performance.

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

No branches or pull requests

3 participants