How to take the some elements of a list of random numbers and sort them?

Question

I want to create a file consisting take rows of distinct numbers in ascending order. They are randomly taken from the first total integer. The file will be used to make an excerpt as discussed here.

I realize that my naming convention for identifiers below is not good but please ignore it for the sake of simplicity.

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

namespace Karls.Students.Excerpting
{
    class Program
    {
        static void Main(string[] args)
        {
            int total = int.Parse(args[0]);
            int take = int.Parse(args[1]);
            int seeder = int.Parse(args[2]);
            string filename = args[3];


            int[] array = Enumerable.Range(1, total).ToArray();

            Random random = new Random(seeder);
            for (int i = total - 1; i > 0; i--)
            {
                int j = random.Next(i+1);
                int temp = array[i];
                array[i] = array[j];
                array[j] = temp;
            }

            File.WriteAllLines(filename, array.Take(take).OrderBy(x => x).Select(x => x.ToString()));
        }
    }
}

With best practice in mind, is there anything "bad" in my code?

Answer 1

Instead of shuffling items yourselves it's easier to apply random sort. So your logic can be rewritten in a single statement:

var random = new Random();

var result = Enumerable.Range(1, total)
    .OrderBy(i => random.Next())
    .Take(take)
    .OrderBy(i => i);

Answer 2

With out commenting on the business logic (the class assignment) I would say this about the coding standard as a whole.

My opionion is: You are writing very c style. Instead think of C# as oo

So Smaller functions, this make it very obvious to anyone reading it whats going on with out trying to understand the actually code. Easier to read = easier to maintain and extend project later on..

Your primary function (usually the public method) should read like an english book.

Also you should check as soon as posible for negetive cases, in theory on top of the function. This helps nesting and mid code aborts.

So with a quick style rewrite (sorry not going to touch the business) something like:

namespace Karls.Students.Excerpting
{
   class Program
   {
    static void Main(string[] args)
    {
        var commandLineArgs = new CommandLineArgs(args);

        var array = CreatePlaceHolderStorage(commandLineArgs.Total);

        Randomize(commandLineArgs.Seeder, array);

        WriteListToFile(array, commandLineArgs.Filename, commandLineArgs.Take);
    }

    private static void Randomize(int seed, int[] array)
    {
        var random = new Random(seed);
        for (int i = array.Count() - 1; i > 0; i--)
        {
            int j = random.Next(i + 1);
            int temp = array[i];
            array[i] = array[j];
            array[j] = temp;
        }
    }

    private static int[] CreatePlaceHolderStorage(int total)
    {
        return Enumerable.Range(1, total).ToArray();
    }


    private static void WriteListToFile(IEnumerable<int> listToPrint, string filename, int sizeToTake)
    {
        File.WriteAllLines(filename, listToPrint.Take(sizeToTake).OrderBy(x => x).Select(x => x.ToString()));
    }
}

public class CommandLineArgs
{
    public int Total { get; set; }
    public int Take { get; set; }
    public int Seeder { get; set; }
    public string Filename { get; set; }

    public CommandLineArgs(string[] args)
    {
        Total = int.Parse(args[0]);
        Take = int.Parse(args[1]);
        Seeder = int.Parse(args[2]);
        Filename = args[3];
    }
}