Created a function with two ints 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

CodePudding user response：

Linq solution just for the fun of it... instructive (allthough overkill) as it shows use of generics, tuples and how you can tweak built-in linq operations with custom comparers.

private static (int Guessed, int Misplaced) CompareRows<T>
    (IEnumerable<T> guesses, IEnumerable<T> master)
    where T: IEquatable<T>
{
    var guessesWithIndex = guesses.Select((g, i) => (g, i));
    var masterWithIndex = master.Select((m, i) => (m, i));

    var guessed = masterWithIndex.Intersect(guessesWithIndex);
    var potentiallyMisplacedGuesses = guessed.Any()?
        guessesWithIndex.Except(guessed): guessesWithIndex;
    var masterNotGuessed = guessed.Any() ?
        masterWithIndex.Except(guessed) : masterWithIndex;
    var misplaced = potentiallyMisplacedGuesses.Intersect(
        masterNotGuessed, new OnlyFirstValueComparer<T, int>());
    return (guessed.Count(), misplaced.Count());
}

class OnlyFirstValueComparer<T, K> : IEqualityComparer<(T, K)> where T: IEquatable<T>
{
    public bool Equals((T, K) x, (T, K) y)
    {
        return x.Item1.Equals(y.Item1);
    }

    public int GetHashCode((T, K) obj)
    {
        return obj.Item1.GetHashCode();
    }
}