Randomised array not written correctly on file

Question

Really new to c#, sorry if its a simple question. When trying to write a randomised user entered array on a file, the input names are not saved correctly.

Code:

using System;
using System.Linq;
using System.IO;

namespace test
{
    class Program
    {
        static void Main(string[] args)
        {
            string[] lines = System.IO.File.ReadAllLines(@"C:\Users\dvfsn\Desktop\Coding\cSharp testing\studentnames.txt");  
            Random rnd = new Random();

            Console.WriteLine("Do you want to read or write data?");
            switch(Console.ReadLine()){
                case "write":
                     string[] names = new string[10];

                    for (int i = 0; i < names.Length; i++) 
                    {
                        Console.WriteLine("Enter names {0}", i);
                        names[i] = Console.ReadLine();
                        names = names.OrderBy(x => rnd.Next()).ToArray();
                        File.WriteAllLinesAsync("studentnames.txt", names);                    
                    }
                break;

                case "read":
                    Console.WriteLine("name of data you want to read?");
                    switch(Console.ReadLine()){
                        case "names":
                            Console.WriteLine("the contents include:");
                            foreach (string line in lines){
                                Console.WriteLine("\t" + line);
                            }
                        break;
                    }
                break;



            }

            Console.ReadLine();
        }
    }
}

Result:

How the data was written on the file(image)

This may be too much to ask but if you could explain the solution that would be really helpful. Thanks

You're writing to the file every time the user adds a name to the array. That seems... unnecessary. Gather the name in the loop, then randomize and write outside of the loop. — Heretic Monkey, Oct 05 '21 at 17:27
Ordering by `rnd.Next()` is not okay. Wait to load all the names, use a [shuffling algorithm](https://stackoverflow.com/questions/273313/randomize-a-listt/1262619#1262619), and then finally write the file **once**. — Joel Coehoorn, Oct 05 '21 at 17:48

Ermiya Eskandary · Answer 1 · 2021-10-05T18:11:25.183

Your output is incorrect as on every .OrderBy call, you are essentially changing the order of the items that have been added to the list. This means that you are theoretically overwriting an existing name every time you add a new name to the array.

The code can also be improved performance wise by writing to the file after all names have been collated instead of after every input.

To fix the output, "shuffle" the list after all names have been added.

Your "shuffling" mechanism will work to just randomise the data but to shuffle the list correctly, opt for an actual shuffling algorithm like Fisher-Yates shuffle.

The question is regarding the file output so I'll leave the shuffling algorithm for you to figure out :) however the below code should result in correct output.

case "write":
    var maxNameCount = 10;
    var names = new string[maxNameCount];

    for (int i = 0; i < maxNameCount; i++)
    {
        Console.WriteLine("Enter names {0}", i);
        names[i] = Console.ReadLine();
    }

    var shuffledNames = names.OrderBy(name => Guid.NewGuid());
    File.WriteAllLinesAsync("studentnames.txt", shuffledNames);

    break;

score 1 · Answer 2 · answered Oct 05 '21 at 17:53

To understand what's going on, think about what happens to the names array after one iteration through the loop:

string[] names = new string[10];

for (int i = 0; i < names.Length; i++) 
{
    Console.WriteLine("Enter names {0}", i);
    names[i] = Console.ReadLine();
    names = names.OrderBy(x => rnd.Next()).ToArray();
    File.WriteAllLinesAsync("studentnames.txt", names);                    
}

After one iteration, we have stored an item in the first element, and then shuffled the array. There is now a random chance the item we just entered is no longer in the first position, but rather later in the array, waiting to be overwritten.

This chance happens again every time we go through the loop. The code will add another item, then move everything around such that we set ourselves up to overwrite the item we just added. It may also move empty, still unwritten items to the early part of the array the code has already passed.

To fix this, wait to shuffle the array (and write the file) until all the items are entered:

string[] names = new string[10];

for (int i = 0; i < names.Length; i++) 
{
    Console.WriteLine("Enter names {0}", i);
    names[i] = Console.ReadLine();                  
}
names = names.OrderBy(x => rnd.Next()).ToArray();
File.WriteAllLinesAsync("studentnames.txt", names);

While I'm here, ordering by rnd.Next() is not good. Instead, you should an actual shuffle. This will be faster, and it will eliminate opportunities for bias.

You forgot to talk about `WriteAllLinesAsync` unless you have some hidden agenda behind it... — Alexei Levenkov, Oct 05 '21 at 18:26

Randomised array not written correctly on file

2 Answers2