Here is my second attempt at writing clean code for "Sherlock and Pairs" challenge. For my previous thread please look here: Calculate pairs in a Set ("Sherlock and Pairs" HackerRank challenge)
Problem statement:
Sherlock is given an array of N integers (A0,A1…AN−1) by Watson. Now Watson asks Sherlock how many different pairs of indices i and j exist such that i is not equal to j but Ai is equal to Aj.
That is, Sherlock has to count the total number of pairs of indices (i,j) where Ai=Aj AND i≠j.
Input Format
The first line contains T, the number of test cases. T test cases follow. Each test case consists of two lines; the first line contains an integer N, the size of array, while the next line contains N space separated integers.
Output Format
For each test case, print the required answer on a different line.
Constraints
1≤T≤10
1≤N≤105
1≤A[i]≤106
Things that I have changed in my code:
- Removed dead code
- Followed naming conventions. Camel case for all local variables.
- Replaced class variables with parameters.
Things that I have kept the same:
- Currently there are no messages being shown in the Console for the user. I have purposely kept it that way as on HackerRank while submitting the solution I cannot post any messages to the console.
Things that still need improvement:
- I wanted to follow the single responsibility principle. However my
CalculatePairsForAllTestCases
function takes input from the user, splits it and then proceeds to the calculation. Can someone help me with how can I split that function further into Single responsibility functions?
using System;
using System.Collections.Generic;
using System.Linq;
namespace NoOfPairsInASet
{
class Solution
{
static void Main()
{
int testCases = Convert.ToInt32(Console.ReadLine());
CalculatePairsForAllTestCases(testCases);
}
private static void CalculatePairsForAllTestCases(int testCases)
{
for (int t = 0; t < testCases; t++)
{
int noOfElements = Convert.ToInt32(Console.ReadLine());
string elements = Console.ReadLine();
string[] arrayOfElements = elements.Split(' ');
long[] data = arrayOfElements.Select(s => long.Parse(s)).ToArray();
long totalPairs = CalculatePairs(data);
Console.WriteLine(totalPairs);
}
}
private static long CalculatePairs(long[] data)
{
Dictionary<long,long> countOfElements = GenerateDictionary(data);
long sum = CalculateSum(countOfElements);
return sum;
}
private static Dictionary<long, long> GenerateDictionary(long[] data)
{
Dictionary<long,long> countOfElements = new Dictionary<long, long>();
const int DEFAULTCOUNT = 1;
for (long i = 0; i < data.Length; i++)
{
if (countOfElements.ContainsKey(data[i]))
{
IncrementCounter(data,countOfElements,i);
}
else
{
countOfElements.Add(data[i], DEFAULTCOUNT);
}
}
return countOfElements;
}
private static void IncrementCounter(long[] data,Dictionary<long, long> countOfElements, long i)
{
if (countOfElements != null) countOfElements[(data[i])]++;
}
private static long CalculateSum(Dictionary<long, long> countOfElements)
{
long sum = 0;
if (countOfElements == null) return sum;
sum = countOfElements.Where(item => item.Value > 1).Sum(item => CalculatePermutationOfValue(item.Value));
return sum;
}
private static long CalculatePermutationOfValue(long n)
{
return n * (n - 1);
}
}
}
Please review the code and let me know how I could have written it better in terms of logic, coding practices or any other advice you might have.