Created a function with two int and two lists. This approach still double counts. Example: row 1: 1,2,2,3 row 2: 1,4,2,5

It shows wrongPosition: 1 and samePlace: 2 instead of samePlace: 2 and wrongPosition:0. Also, do you have any suggestion as to how to optimise this code?

private void CompareRows()
{
if (\_resaultPawns.Length != \_actualRowPawns.Length)
{
Debug.LogError("Arrays have different length");
return;
}
int swrongPosition = 0;
int samePlace = 0;
List\<int\> alreadyCounted = new List\<int\>();
List\<int\> alreadyCountedColors = new List\<int\>();

        // Check for same position with same color
        for (int i = 0; i < _resaultPawns.Length; i  )
        {
            if (_resaultPawns[i].GetComponent<MeshRenderer>().material.color == _actualRowPawns[i].GetComponent<MeshRenderer>().material.color)
            {
                if (!alreadyCounted.Contains(i))
                {
                    samePlace  ;
                    alreadyCounted.Add(i);
                }
            }
        }
    
        // Check for wrong position with same color
        for (int i = 0; i < _resaultPawns.Length; i  )
        {
            for (int j = 0; j < _actualRowPawns.Length; j  )
            {
                if (_resaultPawns[i].GetComponent<MeshRenderer>().material.color == _actualRowPawns[j].GetComponent<MeshRenderer>().material.color && !alreadyCounted.Contains(i) && !alreadyCountedColors.Contains(j))
                {
                    if (!alreadyCounted.Contains(i) && !alreadyCountedColors.Contains(j))
                    {
                        swrongPosition  ;
                        alreadyCountedColors.Add(j);
                        alreadyCounted.Add(i);
                    }
                }
            }
        }

}```

CodePudding user response：

It seems pretty simple to just do this:

var hidden = new[] { 1, 2, 2, 3 };
var first = new[] { 1, 4, 2, 5 };

int sp = 0;
int wp = 0;
for (int i = 0; i < 4; i  )
{
    if (first[i] == hidden[i])
        sp  ;
    else
        for (int j = 0; j < 4; j  )
            if (first[i] == hidden[j] && first[j] != hidden[j])
            {
                wp  ;
                break;
            }
}

Console.WriteLine($"Same Place: {sp}, Wrong Place: {wp}");

That outputs Same Place: 2, Wrong Place: 0.

CodePudding user response：

See https://dotnetfiddle.net/91GVVe

I looked at this from optimizations/maintainability first and refactoring the code a bit seems to already have solved the issue as well ^^

    var samePlace = new List<int>(4);
    var wrongPosition = new List<int>(4);

    for (var i = 0; i < _resaultPawns.Length; i  )
    {
        if (_resaultPawns[i].GetComponent<MeshRenderer>().material.color == _actualRowPawns[i].GetComponent<MeshRenderer>().material.color)
        {
            // Check for 'alreadyCounted.Contains' is redundan at this stage
            
            // No additional counter needed - simply add items and use the list Count later on
            samePlace.Add(i);
        }
    }

    for (var i = 0; i < _resaultPawns.Length; i  )
    {
        // Can directly skip as already counted
        if (samePlace.Contains(i)) continue;
        
        for (var j = 0; j < _actualRowPawns.Length; j  )
        {
            // can directly skip as would have been upper loop match
            // slightly chaper though than using the 'samePlace.Contains(j)' below
            if (i == j) continue;
            
            // skip as well to not re-use already exact matches 
            if (samePlace.Contains(j)) continue;
            
            // skip to not use twice
            if (wrongPosition.Contains(j)) continue;
            
            if (_resaultPawns[i].GetComponent<MeshRenderer>().material.color == _actualRowPawns[j].GetComponent<MeshRenderer>().material.color)
            {
                wrongPosition.Add(j);
                break;
            }
        }
    }

    var wrongPositionCount = wrongPosition.Count;
    var samePlaceCount = samePlace.Count;

I guess the main bit I added was

if (samePlace.Contains(j)) continue;

=> you also want to ignore if a j index was already seen before by the first loop as it is already an exact match.

In your example

1,2,2,3 

1,4,2,5

even though the index 3 (=value 2) was already counted as an exact match it was used again as an alternative position for index 1, a case you didn't check for as it is neither in your alreadyCounted nor in alreadyCountedColour