Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Upgrade checkAnagram function & Fixes #902

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

Merged
raklaptudirm merged 14 commits into TheAlgorithms:master from fahimfaisaal:upgrade-anagram
Feb 25, 2022
Merged

Upgrade checkAnagram function & Fixes #902

raklaptudirm merged 14 commits into TheAlgorithms:master from fahimfaisaal:upgrade-anagram
Feb 25, 2022

Conversation

Copy link
Contributor

@fahimfaisaal fahimfaisaal commented Feb 24, 2022

Welcome to the JavaScript community

Open in Gitpod know more

Describe your change:

  • Optimize the algorithm via Array.prototype.reduce & String.prototype.replace methods
  • Fix a bug case-sensitive to insensitive by using RegExp
  • Add new JS Doc
  • Add new test cases for invalid input & case-insensitive
  • Fixed: Anagram is or not case sensitive! #901

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Copy link

kikar commented Feb 24, 2022
edited
Loading

Optimize the algorithm via Array.prototype.reduce & String.prototype.replace methods

Looks like the algorithm is less efficient now. It was O(n) and now is O(n^2)

@raklaptudirm raklaptudirm linked an issue Feb 24, 2022 that may be closed by this pull request
Copy link
Member

@kikar I think the new implementation can teach more Javascript concepts than the previous one, which is our primary aim. That said, I think it will be better to have the two implementations separately @fahimfaisaal.

kikar reacted with thumbs up emoji

Copy link
Contributor Author

@kikar to my knowledge, the String.prototype.replace method used regex that's why the operation is very cheap, cause it's not traversing the full string str2Acc in every traverse of Array.prototype.reduce method. it just removes 1 char from str2Acc in 1 Array.prototype.reduce method traverse. which char is just matched to the cur variable.

So, the algorithm couldn't be O(n^2) complexity.

Copy link

kikar commented Feb 24, 2022

@kikar to my knowledge, the String.prototype.replace method used regex that's why the operation is very cheap, cause it's not traversing the full string str2Acc in every traverse of Array.prototype.reduce method. it just removes 1 char from str2Acc in 1 Array.prototype.reduce method traverse. which char is just matched to the cur variable.

So, the algorithm couldn't be O(n^2) complexity.

Even if use regex, the code needs to find the first occurrence of the letter selected. Take the case of the reversed string.
For each letter from str1 you need to traverse the whole str2 to find the letter to remove.

fahimfaisaal reacted with thumbs up emoji

Copy link
Contributor Author

@kikar to my knowledge, the String.prototype.replace method used regex that's why the operation is very cheap, cause it's not traversing the full string str2Acc in every traverse of Array.prototype.reduce method. it just removes 1 char from str2Acc in 1 Array.prototype.reduce method traverse. which char is just matched to the cur variable.
So, the algorithm couldn't be O(n^2) complexity.

Even if use regex, the code needs to find the first occurrence of the letter selected. Take the case of the reversed string. For each letter from str1 you need to traverse the whole str2 to find the letter to remove.

Yes, It's definitely not O(1) but ES6 doesn't run the search algorithm:

Search string for the first occurrence of searchString and let pos be the index within a string of the first code unit of the matched substring and let matched be searchString. If no occurrences of searchString were found, return string.

but in the long string, it will be slower.

Copy link
Contributor Author

@kikar I think the new implementation can teach more Javascript concepts than the previous one, which is our primary aim. That said, I think it will be better to have the two implementations separately @fahimfaisaal.

@raklaptudirm I think if I optimize my algorithm, It's shouldn't add two more implementations. please let me know what could I do.

Copy link
Member

Since the two implementations are very different, I would suggest to keep and test both.

Copy link
Contributor Author

@raklaptudirm I reckon, if we add this algorithm it's will replace previous algos.

const str1List = Array.from(str1.toUpperCase()) // str1 to array 
 // get the occurrences of characters of str1 by using HashMap
 const str1Occurs = str1List.reduce(
 (map, char) => (
 map.set(char, map.get(char) + 1 || 1), map
 ),
 new Map()
 )
 for (const char of str2.toUpperCase()) {
 // if char has not exist to the map it's return false
 if (!str1Occurs.has(char)) {
 return false;
 }
 let getCharCount = str1Occurs.get(char)
 str1Occurs.set(char, --getCharCount)
 getCharCount === 0 && str1Occurs.delete(char)
 }
 return true;
}

Copy link
Member

Both do not need to have the same name.

Copy link
Contributor Author

Ok I will add these two with different names, but could I add these two algos in One file?

Copy link
Member

Yes.

fahimfaisaal reacted with thumbs up emoji

Copy link
Contributor Author

Ok, thanks for your rapid response.

raklaptudirm and fahimfaisaal reacted with laugh emoji

Copy link
Member

@raklaptudirm raklaptudirm left a comment

Choose a reason for hiding this comment

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

Rename the functions to remove via from the name.

Copy link
Contributor Author

You're really strict reviewer. 😀

@raklaptudirm raklaptudirm changed the title (削除) merge: Upgrade checkAnagram function & Fixes: #901 (削除ここまで) (追記) Upgrade checkAnagram function & Fixes (追記ここまで) Feb 25, 2022
@raklaptudirm raklaptudirm merged commit d466be9 into TheAlgorithms:master Feb 25, 2022
@fahimfaisaal fahimfaisaal deleted the upgrade-anagram branch March 29, 2022 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raklaptudirm raklaptudirm raklaptudirm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Anagram is or not case sensitive!

AltStyle によって変換されたページ (->オリジナル) /