For loop changes all elements in the array, rather than just one?

Question

I'm using a for loop to read the lines of a file one by one. I then use the data in each line of the file to change the properties of the objects in an array.

I've run through the code in Debug mode, and it all seems to run fine. It reads the line of the file which corresponds to the i value of the for loop correctly, it defines an array of this data based on the commas, and it creates a temporary object which stores these values in the correct format. I know all of this because, as I've said, I have checked the values in Debug mode.

However, on the last line inside of the loop it seems to change every element in the array to the values stored in scrOldScore, whereas I want it to keep the values that it read in from previous lines and just update the element of the array corresponding to the i of the for loop.

With each iteration of the for loop, the array holds identical data in each element that isn't null, and with each iteration that data changes to the most recently defined scrOldScore.

string str;
Data d = new Data();

for (int i = 0; i < arr.Length; i++)
{
    str = File.ReadLines(FileName).Skip(i).Take(1).First(); ;
    string[] string = new string[4];
    string = str.Split(',');
    
    d.Property01 = string[0];
    d.Property02 = Convert.ToInt32(string[1]);
    d.Property03 = string[2];
    d.Property04 = string[3];
    arr[i] = d;
}

Thanks for any help :)

Answer 1

The code doesn't work because it's putting the same ScoreData object instance in every array item. You need to create a new ScoreData object instance inside the loop.

But also the original code re-opens and reads through the file a little further on every iteration of the loop. This is grossly inefficient. You can make things run in a fraction of the time by keeping the same file handle, which you can do by using the file, rather than the array, as the main item for the loop:

int i = 0;
var lines = File.ReadLines(strFileName);

foreach(var line in lines)
{
    var data = line.Split(',');
    ScrScores[i] = new ScoreData() {
        Name = data[0],
        Score = int.Parse(data[1]),
        Accuracy = data[2],
        ReactionTime = data[3]
    };
    i++;
}

Of course, there's a chance here the file might be larger than the array. If that's possible, you should probably be using a List<ScoreData> rather than an array anyway:

var ScrScores = new List<ScrScores>();
var lines = File.ReadLines(FileName);
foreach(var line in lines)
{
    var data = line.Split(',');
    ScrScores.Add(new ScoreData() {
        Name = data[0],
        Score = int.Parse(data[1]),
        Accuracy = data[2],
        ReactionTime = data[3]
    });
}

Failing both of these, I would open a StreamReader object, rather than using File.ReadLines(), and call it's ReadLine() method in each for loop iteration.

using (var rdr = new StreamReader(FileName))
{
    for (int i = 0; i < ScrScores.Length; i++)
    {
        var data = rdr.ReadLine().Split(',');
        ScrScores[i] = new ScoreData() {
            Name = data[0],
            Score = int.Parse(data[1]),
            Accuracy = data[2],
            ReactionTime = data[3]
        };
    }
}

One last thing we can do, since it looks like we're replacing all the elements in an array of known size, is replace the entire array. To do this we can read exactly that many items from the file and project them into ScoreData objects as we go:

ScrScores = File.ReadLines(FileName).
               Select(line => line.Split(',')).
               Select(data => new ScoreData() {
                  Name = data[0],
                  Score = int.Parse(data[1]),
                  Accuracy = data[2],
                  ReactionTime = data[3]
               }).
               Take(ScrScores.Length).
               ToArray();

Technically, this is one line of code (only one semi-colon and could be extended to fit on the same line).

I'll add that hungarian notation variable prefixes like str and arr are a hold-over from the VB6 era. Today, tooling has improved and even Microsoft's own coding guidelines — where the practice originated — now specifically say (in bold type, no less) "Do not use Hungarian notation". All the code examples I provided reflect this recommendation.