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

Possible optimization? #140

Open
gdetari opened this issue Sep 23, 2024 · 2 comments
Open

Possible optimization? #140

gdetari opened this issue Sep 23, 2024 · 2 comments

Comments

@gdetari
Copy link
Contributor

gdetari commented Sep 23, 2024

In the following function:

private HashSet<string> Edits(string word, int editDistance, HashSet<string> deleteWords)
    {
        editDistance++;
        if (word.Length > 1)
        {
            for (int i = 0; i < word.Length; i++)
            {
                string delete = word.Remove(i, 1);
                if (deleteWords.Add(delete))
                {
                    //recursion, if maximum edit distance not yet reached
                    if (editDistance < maxDictionaryEditDistance) Edits(delete, editDistance, deleteWords);
                }
            }
        }
        return deleteWords;
    }

Would it make sense to only add to calculate and add the words to deleteWords if editDistance < maxDictionaryEditDistance is true? That is quite a difference in loading dictionary performance. It does change the functionality, but isn't that the correct approach not to add the words anyhow?

@wolfgarbe
Copy link
Owner

I think we already do exactly this. Edits is a recursive method.
Only in the first iteration we unconditionally calculate and add deletes.
That is because we always have maxDictionaryEditDistance>=1.
But the second and following iterations of Edits are done only if editDistance < maxDictionaryEditDistance.

@gdetari
Copy link
Contributor Author

gdetari commented Sep 24, 2024

Yes, what I meant, that when editDistance == maxDictionaryEditDistance, we do add the delete word, but not call the recursion anymore. I was wondering if we need to add the delete word at all at this case, but now that I pondered on it more, I think it is intentional, right? That case should still be included, just not going anymore deeper with the recursion.

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

2 participants