4

Here I add something to a Dictionary:

dictionary.Add(dictionaryKey, value);

if dictionaryKey already exists, an ArgumentException will be thrown. Its message is fairly generic:

An item with the same key has already been added.

If my call to dictionary.Add is inside of a loop or a helper function, it can be hard to tell immediately what key has already been added that is throwing this exception. I would like to know this as easily and as soon as possible.

There are a few options.

1)

if(dictionary.ContainsKey(dictionaryKey)
{
    throw new ArgumentException($"An item with the same key ({dictionaryKey}) has already been added.");
}

dictionary.Add(dictionaryKey, value);

2)

try
{
    dictionary.Add(dictionaryKey, value);
}
catch(ArgumentException argumentException)
{
    throw new ArgumentException($"An item with the same key ({dictionaryKey}) has already been added.");
}

3) Some other way

I know setting up a try/catch block takes a performance hit, but it seems running dictionary.ContainsKey(dictionaryKey) would mean an additional lookup each time too. Which of these options is most performant?

Scotty H
  • 6,432
  • 6
  • 41
  • 94
  • 1
    I would go with first approach as dictionary contains has only O(1) complexity. Exception handling should not be used in such case. – Akash KC Jun 30 '17 at 15:52
  • 2
    Try/catch blocks don't provoke any performance hit at all, it's the throwing of an exception that does – Camilo Terevinto Jun 30 '17 at 15:56
  • Shouldn't there already be an indication in the error message of what was being inserted? – jth41 Jun 30 '17 at 16:18

2 Answers2

2

Not sure where the context of this code falls into, but in terms of performance, it will depend if you are expecting duplicate dictionaryKeys.

If there are going to duplicates, I would go with the first method, as ContainsKey is an O(1) operation, while try/catch incurs a small performance penalty if used to handle control flow. Presumably, this penalty will be greater than O(1).

However if you can guarantee no duplicate dictionaryKeys, the second method would be faster. The try/catch performance penalty only occurs if an exception is thrown (a duplicate key is found). The first method will be performing an unnecessary call to ContainsKey. Of course, this means you don't need to wrap the code in a try/catch in the first place, defeating the purpose of your question.

I would go with the first method.

budi
  • 6,351
  • 10
  • 55
  • 80
0

I would say number one is better. Because when you use the second method it will trigger an runtime error and that means it will create an additional Exception object in the memory which referenced by the variable "argumentException" in the catch phrase.

Lasitha Lakmal
  • 486
  • 4
  • 17