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

IsAnagram bug (per element counts) #153

Open
ZacharyPatten opened this issue Jan 17, 2021 · 2 comments
Open

IsAnagram bug (per element counts) #153

ZacharyPatten opened this issue Jan 17, 2021 · 2 comments

Comments

@ZacharyPatten
Copy link

ZacharyPatten commented Jan 17, 2021

Describe the bug

The IsAnagram function is not checking per-element counts. It is only checking if they have the same elements, but not if the count of each element matches.

A more appropriate name for the current logic is something like ContainsNoDifferingElements or IntersectsMatch rather than IsAnagram. I would recommend changing the name or the logic of the method.

Note: If you aren't going to check per-element counts, then you should also get rid of this check in IsAnagrams:

if (source.Length != other.Length)
    return false;

because length doesn't matter if you don't also check per-element counts.

To Reproduce

Add the following case to the IsAnagram unit tests:

string aab = "aab";
string abb = "abb";
Assert.False(Permutations.IsAnargram(aab, abb));

Expected behavior

Spans of the same length and elements but different per-element counts should not be considered re-orders/anagrams of each other.

Environment:

master branch

Additional context

I have written my own version of this algorithm in C# (that fixes this issue) if interested here...

Source Code: https://github.com/ZacharyPatten/Towel/blob/d2660e208ad3a44ab22f192834760c5b93dc82ac/Sources/Towel/Statics-SequenceAnalysis.cs#L1321
Examples: https://github.com/ZacharyPatten/Towel/blob/d2660e208ad3a44ab22f192834760c5b93dc82ac/Examples/BasicsAndExtensions/Program.cs#L406
Testing: https://github.com/ZacharyPatten/Towel/blob/d2660e208ad3a44ab22f192834760c5b93dc82ac/Tools/Towel_Testing/Statics.cs#L2086
Note: MapHashLinked is my version of a Dictionary if you look at the source code.

@ZacharyPatten
Copy link
Author

related to #11

@github-actions
Copy link

Thanks for supporting the development of C# Algorithms with your first issue! We look forward to handling it.

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

1 participant