VSTO Write Dictionary To Range Extension Method

Question

I was hoping to receive some criticism on this block of code. I wrote it to write the contents of a dictionary to excel range in VSTO. It works well. I am interested to see how people could improve it. What would you do differently to make it more expressive, efficient?

public static void WriteToRange(this Dictionary<string, object[]> DicToRange, excel.Range toRange)
    {
       object[,] storeDictionaryData = new object[DicToRange.Max(maxValues=> maxValues.Value.Length),DicToRange.Count];

       int keyCounter=0;
       int valueCounter =0;



       foreach (var key in DicToRange.Keys)
       {
         if (keyCounter < storeDictionaryData.GetLength(1))
           {
               foreach (var value in DicToRange[key])
               {
                   if (valueCounter < storeDictionaryData.GetLength(0))
                   {
                       storeDictionaryData[valueCounter, keyCounter] = value;
                       valueCounter++;
                   }
               }

               keyCounter++;
               valueCounter = 0;
           }
       }

       dynamic rangeToFill=toRange.get_Resize(storeDictionaryData.GetLength(0), storeDictionaryData.GetLength(1));
       rangeToFill.Value = storeDictionaryData;
    }

Answer 1

You have a lot of unnecessary vertical spacing which should be removed to improve readability.

---

Based on the naming guidelines input parameter should be named using camelCase casing.

---

Because WriteToRange() can be called also as a normal static method you need to check both input parameters for null.

---

The storeDictionaryData is basically a object[columns, rows] this should be reflected by the counter variables. You can just rename them valueCounter -> column, keyCounter -> row.

---

You don't need the checks keyCounter < storeDictionaryData.GetLength(1) and valueCounter < storeDictionaryData.GetLength(0) because at the point where you create the array you have taken care of the upper bounds of both dimensions. Both conditions won't evaluate to false.

---

toRange is poorly named. Consider to name it destinationRange or simply destination. Then a good pattern would be also to rename DictToRange to source.

---

You should allow your variables to breathe by adding spaces between variables and operators.

---

Taking the above points into account we can refactor the method to

public static void WriteToRange(this Dictionary<string, object[]> source, excel.Range destination)
{
   if (source == null) { throw new ArgumentNullException("source"); }
   if (destination == null) { throw new ArgumentNullException("destination"); }
   if (source.Count == 0) { throw new ArgumentOutOfRangeException("source"); }

   object[,] storeDictionaryData = new object[source.Max(maxValues => maxValues.Value.Length), source.Count];

   int column = 0;
   int row = 0;

   foreach (var key in source.Keys)
   {
       foreach (var value in source[key])
       {
           storeDictionaryData[column, row] = value;
           column++;
       }

       row++;
       column = 0;
   }

   dynamic rangeToFill = destination.get_Resize(storeDictionaryData.GetLength(0), storeDictionaryData.GetLength(1));
   rangeToFill.Value = storeDictionaryData;
}

Answer 2

Adding to @Heslacher's excellent review, I would say that there is one thing in the signature that tickles reusability and extensibility:

public static void WriteToRange(this Dictionary<string, object[]> DicToRange, excel.Range toRange)

Do you see it?

That's right: this Dictionary<string, object[]> is the main reason for missing a heartbeat here.

Extend interfaces! You're extending a specific implementation of the System.Collections.Generic.IDictionary<TKey,TValue> interface. This means I can't use this extension method on a ConcurrentDictionary<TKey,TValue>, or a Sorted<TKey,TValue>, or any other possible implementation of IDictionary<TKey,TValue> ever written - and more importantly, those not written yet, too.

By extending a concrete type, you're... missing out on the best of extension methods.