3
\$\begingroup\$

I have two different arrays that are widely different in length, arrayA, arrayB

for example:arrayA.Length=1000 and arrayB.Length=100,000

Create a boolean function that returns true if each item in arrayA are also present in arrayB, at least the same number of times as in arrayA.

Please review my code below and help me with siuggestions to improve this code below

    private static bool IsMinRequiredFrequencyMatch(int[] arrA,int[] arrB)
    {
    bool result = false;        
    Array.Sort(arrA);

    //create dictionary from shorter array 
    var dict = new Dictionary<int, int>();
    for (int i = 0; i < arrA.Length; i++)
    {
        var ch = arrA[i];
        if (!dict.ContainsKey(ch))
        {
            dict[ch] = 1; //add char count
        }
        else
        {
            dict[ch]++; //increase count
        }
    }




    //loop through Dictionary
    foreach (var kvp in dict)
    {
        var chr = kvp.Key; //target item to find
        var expectedcount = kvp.Value; //expected count of item in arrB
        int actualCount = 0; //actual count of item in arrB

        //loop through arrayB
        for (int i = 0; i < arrB.Length; i++)
        {
            if (arrB[i] == chr)
            {
                actualCount++;
            }

            if (actualCount >= expectedcount)
            {
                result = true;
                break;
            }
        }

        if (actualCount < expectedcount)
        {
            result = false;             
            break;
        }
    }

    return result;


}
\$\endgroup\$
2
  • \$\begingroup\$ 1. "result = false;" followed by "break;" does not make much sense to me. Why don't it just "return false;"? 2. chr, i, kvp, ch, and dict are horrible names that make reader think too much about unnecessary stuff. \$\endgroup\$ Apr 5 '17 at 4:55
  • \$\begingroup\$ @IgorSoloydenko 1) if actualCount >= expectedcount, then the condition for the item is already met, then there is no need to check and count the occurrence of the same element especially when the arrayB size is very long, that's why I added "break" 2) agreed \$\endgroup\$
    – roney
    Apr 5 '17 at 5:46
2
\$\begingroup\$

Sure, you can always write your own method for everything but if you are lazy you could just use LINQ with ToLookup + All + Count

var arr1 = new[] { 1, 2, 3, 4, 3, 2 };
var arr2 = new[] { 6, 7, 3, 4, 1, 2, 2, 3 };

var lookup1 = arr1.ToLookup(x => x);
var lookup2 = arr2.ToLookup(x => x);

var isFullyContained = lookup1.All(x => lookup2[x.Key].Count() == x.Count());

false - means arr1 is fully contained in arr2

The good thing about lookup is that you don't need the ContainsKey method because if the key does not exist it'll return an empty collection.


Review

You create a dictionary only for the first array but you need one for the second array too or otherwise you need to loop over it each time which is unnecessary. Just count the items and compare their counts.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.