patterncsharpMinor
Writing an anagram efficiently
Viewed 0 times
anagramefficientlywriting
Problem
I know this question has been asked before now but in Python. Recently, I set out to write an anagram in C#. For instance, orchestra can be rearranged into carthorse and the anagram should not have more repeated than the original. The function is meant to check if the two words are the same and return true in this case. I started with putting the two words in two separate dictionaries with the letters as a key and the count as value.
The idea is using a dictionary ensures the keys can not be repeated. Then I looped through the second dictionary (
```
using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary firstString = new Dictionary();
Dictionary secondString = new Dictionary();
foreach(char character in a)
{
if(firstString.ContainsKey(character)== true)
{
firstString[character]+=1;
}
else
firstString[character]=1;
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2)== true)
{
secondString[character2]+=1;
}
else
secondString[character2]=1;
}
foreach(KeyValuePair letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}
//throw new NotImplementedException("Waiting to be implemented.");
return true;
The idea is using a dictionary ensures the keys can not be repeated. Then I looped through the second dictionary (
secondString) and checked if each letter existed as a key in the first dictionary (firstString). Additionally, if the letter existed, the code checks if they have the same count. The rest of the code is self-explanatory.```
using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary firstString = new Dictionary();
Dictionary secondString = new Dictionary();
foreach(char character in a)
{
if(firstString.ContainsKey(character)== true)
{
firstString[character]+=1;
}
else
firstString[character]=1;
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2)== true)
{
secondString[character2]+=1;
}
else
secondString[character2]=1;
}
foreach(KeyValuePair letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}
//throw new NotImplementedException("Waiting to be implemented.");
return true;
Solution
I took your new code and cleaned it up a bit by
I took your code
and refactored it to this (there is an issue with the if then statement inside last for loop, which I address later)
I also decided to change this so that there are two separate if statements, this will reduce the O(n) in the optimistic case
(Your code cleaned up)
New Code with less complexity
You could merge these into a single if statement, but for the sake of reading I left them separate here.
If one doesn't contain the key of the other one, you want to return false, no need to check the other condition.
Another overall thing that you could do to reduce the O(n) is to evaluate if the strings are the same length, immediately and return false if they are not, because you already know they cannot be anagrams of each other.
like this
then you can remove the check later in the code.
I did that, and also changed
Here is the code that works, I test
- adjusting bracket location
- removed unnecessary nesting of if statements
- gave some operations some breathing room
- fixed some odd indentations
I took your code
using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary firstString = new Dictionary();
Dictionary secondString = new Dictionary();
foreach(char character in a)
{
if(firstString.ContainsKey(character))
{
firstString[character]+=1;
}
else{
firstString[character]=1;
}
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2))
{
secondString[character2]+=1;
}
else{
secondString[character2]=1;
}
}
if(firstString.Count !=secondString.Count){
return false;
}
else {
foreach(KeyValuePair letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}
}
return true;
}
public static void Main(string[] args)
{
Console.WriteLine(AreStringsAnagrams("momdad", "dadmom"));
}
}and refactored it to this (there is an issue with the if then statement inside last for loop, which I address later)
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary firstString = new Dictionary();
Dictionary secondString = new Dictionary();
foreach(char character in a)
{
if(firstString.ContainsKey(character))
{
firstString[character] += 1;
}
else
{
firstString[character] = 1;
}
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2))
{
secondString[character2] += 1;
}
else
{
secondString[character2] = 1;
}
}
if(firstString.Count != secondString.Count)
{
return false;
}
else
{
foreach(KeyValuePair letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key)
&& (firstString[letterValue.Key] != secondString[letterValue.Key]))
{
return false;
}
else
{
return false;
}
}
}
return true;
}
public static void Main(string[] args)
{
Console.WriteLine(AreStringsAnagrams("momdad", "dadmom"));
}
}I also decided to change this so that there are two separate if statements, this will reduce the O(n) in the optimistic case
(Your code cleaned up)
foreach(KeyValuePair letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}New Code with less complexity
foreach(KeyValuePair letterValue in secondString)
{
if (!firstString.ContainsKey(letterValue.Key))
{
return false;
}
if (firstString[letterValue.Key] != secondString[letterValue.Key])
{
return false;
}
}You could merge these into a single if statement, but for the sake of reading I left them separate here.
If one doesn't contain the key of the other one, you want to return false, no need to check the other condition.
Another overall thing that you could do to reduce the O(n) is to evaluate if the strings are the same length, immediately and return false if they are not, because you already know they cannot be anagrams of each other.
like this
using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
if (a.Length != b.Length) {
return false;
}
Dictionary firstString = new Dictionary();
Dictionary secondString = new Dictionary();
/// ...then you can remove the check later in the code.
I did that, and also changed
+= 1 to ++ twice in your code. I also made it case insensitive, but sentences will not work.Here is the code that works, I test
Code Snippets
using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary<char,int> firstString = new Dictionary<char,int>();
Dictionary<char,int> secondString = new Dictionary<char,int>();
foreach(char character in a)
{
if(firstString.ContainsKey(character))
{
firstString[character]+=1;
}
else{
firstString[character]=1;
}
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2))
{
secondString[character2]+=1;
}
else{
secondString[character2]=1;
}
}
if(firstString.Count !=secondString.Count){
return false;
}
else {
foreach(KeyValuePair<char,int> letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}
}
return true;
}
public static void Main(string[] args)
{
Console.WriteLine(AreStringsAnagrams("momdad", "dadmom"));
}
}public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
Dictionary<char,int> firstString = new Dictionary<char,int>();
Dictionary<char,int> secondString = new Dictionary<char,int>();
foreach(char character in a)
{
if(firstString.ContainsKey(character))
{
firstString[character] += 1;
}
else
{
firstString[character] = 1;
}
}
foreach(char character2 in b)
{
if(secondString.ContainsKey(character2))
{
secondString[character2] += 1;
}
else
{
secondString[character2] = 1;
}
}
if(firstString.Count != secondString.Count)
{
return false;
}
else
{
foreach(KeyValuePair<char,int> letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key)
&& (firstString[letterValue.Key] != secondString[letterValue.Key]))
{
return false;
}
else
{
return false;
}
}
}
return true;
}
public static void Main(string[] args)
{
Console.WriteLine(AreStringsAnagrams("momdad", "dadmom"));
}
}foreach(KeyValuePair<char,int> letterValue in secondString)
{
if(firstString.ContainsKey(letterValue.Key))
{
if(firstString[letterValue.Key] != secondString[letterValue.Key]){
return false;
}
}
else
{
return false;
}
}foreach(KeyValuePair<char,int> letterValue in secondString)
{
if (!firstString.ContainsKey(letterValue.Key))
{
return false;
}
if (firstString[letterValue.Key] != secondString[letterValue.Key])
{
return false;
}
}using System;
using System.Collections.Generic;
public class AreAnagrams
{
public static bool AreStringsAnagrams(string a, string b)
{
if (a.Length != b.Length) {
return false;
}
Dictionary<char,int> firstString = new Dictionary<char,int>();
Dictionary<char,int> secondString = new Dictionary<char,int>();
/// ...Context
StackExchange Code Review Q#127580, answer score: 6
Revisions (0)
No revisions yet.