Thread Safe Objects in CSharp - ConcurrentDictionary

I'm using ConcurrentDictionary to hold the records.. I'm reading files from the local system in parallel, to speed up the process.

Sample 1:

static ConcurrentDictionary concurrentDictionary = new ConcurrentDictionary();

private void ReadFilesFromDirectory()
{
  string[] docNames = Directory.GetFiles(CompleteLettersDirectory);
  Parallel.ForEach(docNames, new ParallelOptions { MaxDegreeOfParallelism = 8 }, file =>
    {
      try
      {
          int64 letterId = Convert.ToInt64(file.Split('_')[1]);
          Byte[] CompleteLetterContent;
          CompleteLetterContent = ConvertToPdfWithAspose(System.IO.File.ReadAllBytes(file), letterId);
          concurrentDictionary.GetOrAdd(letterId, CompleteLetterContent);
      }
      catch (Exception ex)
      {
          //Logger.WriteFile(letterId.ToString() + Environment.NewLine, ex);
      }
    }
}

Sample 2: In this sample I want to keep multiple values in dictionary as such I created a letter class which has multiple properties. As Letter class is not threadsafe, will it create any issues ?

```
static ConcurrentDictionary concurrentDictionary = new ConcurrentDictionary();

private void ReadFilesFromDirectory()
{
string[] docNames = Directory.GetFiles(CompleteLettersDirectory);
Parallel.ForEach(docNames, new ParallelOptions { MaxDegreeOfParallelism = 8 }, file =>
{
try
{
int64 letterId = Convert.ToInt64(file.Split('_')[1]);
Byte[] LetterContent;
Byte[] CompleteLetter;

Letter letter = new Letter();
letter.LetterId = letterId;

if(file.Contains("CL"))
{
letter.CompleteLetter = ConvertToPdfWithAspose(System.IO.File.ReadAllBytes(file), letterId);
}
else
{
letter.LetterContent = ConvertToHTML(System.IO.File.ReadAllBytes(file), letterId);
}
concurrentDictionary.GetOrAdd(letterId, letter);
}
catch (Exception ex)
{
//Logger.WriteFile(letterId.ToString() + Environment.

I'm reading files from the local system in parallel, to speed up the process.

Are you sure it's actually beneficial? Hard disks work best when you read them in sequence, not when they have to go back and forth to satisfy concurrent requests. (And even if you have a SSD, you're not going to gain anything.)

You seem to be in the habit of using framework names for types (e.g. Byte, Int64) instead of the C# names (byte, long). I believe that using the C# names is more common.

Byte[] CompleteLetterContent;
CompleteLetterContent = ConvertToPdfWithAspose(System.IO.File.ReadAllBytes(file), letterId);

There is no reason to separate the declaration and assignment here. Also, local variables are generally written in camelCase:

Byte[] completeLetterContent = ConvertToPdfWithAspose(System.IO.File.ReadAllBytes(file), letterId);

System.IO.File.ReadAllBytes(file)

You're already using System.IO, so the namespace should be redundant here. And if it isn't because you have something else called File, then you should probably fix that conflict.

concurrentDictionary.GetOrAdd(letterId, CompleteLetterContent);

You're calling GetOrAdd(), but you always only want to add, you never want to get. For that, you can either use TryAdd() or the indexer setter:

concurrentDictionary.TryAdd(letterId, CompleteLetterContent);
concurrentDictionary[letterId] = CompleteLetterContent;

In all three cases, the behavior is the same if there are no values with the same key, but I think the latter two are clearer.

Byte[] LetterContent;
      Byte[] CompleteLetter;

These variables are never used, get rid of them.

Letter letter = new Letter();
letter.LetterId = letterId;

You can use var (since the type is clear from the assignment) and object initializer here:

var letter = new Letter { LetterId = letterId };

if(file.Contains("CL"))
{
    letter.CompleteLetter = ConvertToPdfWithAspose(System.IO.File.ReadAllBytes(file), letterId);
}
else
{
    letter.LetterContent = ConvertToHTML(System.IO.File.ReadAllBytes(file), letterId);
}

In both branches, you're repeating the System.IO.File.ReadAllBytes(file) part. Consider extracting it into a local variable before the condition.

The code you have shown appears to be thread-safe, assuming that the methods that you're calling from it (ConvertToPdfWithAspose, ConvertToHTML) are thread-safe.

StackExchange Code Review Q#81809, answer score: 2