C# multithreading console app

I making simple multithreading signature program (it calculate SHA256 hash for parts of file). Please check my code and I would be grateful if you tell me about errors in my code

Program.cs

```
using System;
using System.IO;

namespace Signature
{
class Program
{
public static TaskQueue Tasks;
public static HashPrinter Printer;

static void Main(string[] args)
{
if (args.Length < 2)
{
Console.WriteLine("You must specify two parameters. Example: Signature.exe [filename] [chunkSize]");
return;
}
var filename = args[0];
int chunk;
if (!int.TryParse(args[1], out chunk))
{
Console.WriteLine("You shoul specify number as [chunkSize]");
return;
}
try
{
Tasks = new TaskQueue(Environment.ProcessorCount);
Printer = new HashPrinter(200000);
var reader = new FileReader(filename, chunk);
reader.Read();
}
catch (IndexOutOfRangeException ex)
{
PrintError(ex.Message, ex.StackTrace);
}
catch (FileNotFoundException ioException)
{
PrintError(ioException.Message, ioException.StackTrace);
}
catch (DirectoryNotFoundException ioException)
{
PrintError(ioException.Message, ioException.StackTrace);
}
catch (IOException ioException)
{
PrintError(ioException.Message,ioException.StackTrace);
}
catch (Exception err)
{
PrintError(err.Message,err.StackTrace);
}
Console.ReadLine();
}

private static void PrintError(string message,string trace)
{
Console.WriteLine(message);
Console.WriteLi

Program

This is bad

public static TaskQueue Tasks;
public static HashPrinter Printer;

I guess it is used in this way (a public static field) so it can be accessed from each other. A better way would be to inject (pass it e.g as a parameter to the constructor) into the classes which are using it.

If the excption's message and stacktrace is only printed/exported, then there is no need to distinguish between the types of the exception. This is just boilerplate code which isn't needed. The catch part should be reduced to

catch (Exception err)
{
    PrintError(err.Message,err.StackTrace);
}

TaskQueue ctor

if (threadsCount <= 0)
    throw new IndexOutOfRangeException("Processor count should be positive");

From the documentation


The exception that is thrown when an attempt is made to access an element of an array or collection with an index that is outside its bounds.

It seems this won't fit here, will it ? The correct exception which should be thrown is either an ArgumentException or better an ArgumentOutOfRangeException. Both of this exceptions signal to the caller that a passed argument has a problem in some way, which is what you want to tell.

Btw, omitting braces {} although they might be optional can lead to serious bugs so I want to encourage you to always use them.

All of the class level variables which won't change are readonly which is good, but one lonely Queue> _tasks isn't. Is there a reason for this ?

Why is the ByteArrayToString() method public? In addition

int i;
    for (i = 0; i < array.Length; i++)

the int i could be in the loop declaration like so

for (int i = 0; i < array.Length; i++)

its more idiomatic.

While we are at this method, I noticed that the same method is also in the HashPrinter class and therfor should be extracted to a separate class which can be static and used by both classes. You could also turn it into a extension method.

Instead of explicitly using the concrete type one should use var if the right hand side of an assignment makes the type clear.

E.g

private void Consume()
{
    while (true)
    {
        KeyValuePair task = new KeyValuePair();

should be

private void Consume()
{
    while (true)
    {
        var task = new KeyValuePair();

There is more to say, like Dispose() pattern, CancellationTokenSource for cancellation of a thread, consistentness of style, like sometimes C#6 features are used and sometimes not (Console.WriteLine("Part #{0}, Hash: {1}", part.Key, ByteArrayToString(part.Value));).

StackExchange Code Review Q#145249, answer score: 2