0

I have 2 pieces of code. What would say is best practice and why? In the first version I add a new shop and evaluate if it's successful with Exceptions, and in the second version I do the same with "if" statements.

I've recently read that Exceptions can be expensive in the code, and if all "Add" method is implemented this way it can cause performance issues. Can anybody confirm this?

Thanks!!!

public bool AddShop1(Shop newShop)
        {
            try
            {
                ShopDictionary.Add(newShop.ShopId, newShop);
                return true;
            }

            catch (ArgumentNullException)
            {
                return false;
            }

            catch (ArgumentException)
            {
                return false;
            }

        }

public bool AddShop2(Shop newShop)
        {
            if (newShop == null || ShopDictionary.ContainsKey(newShop.ShopId))
            {
                return false;
            }

            ShopDictionary.Add(newShop.ShopId, newShop);
            return true;
        }
CodeCaster
  • 147,647
  • 23
  • 218
  • 272
Soni
  • 19
  • 1
  • 10

3 Answers3

1

you should not use try catch for the control flow logic.

that should be done using if or switches etc.

try catch should be used for uncertain situations only, and should then be handled accordingly.

M. Ali Iftikhar
  • 3,125
  • 2
  • 26
  • 36
0

It depends from how often your function will cause exception. If exceptions will happen rarely (once per 1000 calls), then you can ignore slowdown of performance. But if from 1000 executions you'll get 800 exceptions, then definitely convert to ifs. But then just a question, if from 1000 you have 800 exceptions, is it really exception, or a rule of business logic?

Yuriy Zaletskyy
  • 4,983
  • 5
  • 34
  • 54
-2

Well, avoiding exception is more preferred for the performance for your application. and if you really interested in exception, you can do like this way,

public bool AddShop2(Shop newShop, out Exception exception)
    {
        exception = null;
        try
        {
            if (newShop == null || ShopDictionary.ContainsKey(newShop.ShopId))
            {
                return false;
            }

            ShopDictionary.Add(newShop.ShopId, newShop);
        }
        catch (Exception ex)
        {
            exception = ex;
        }
        return true;
    }
Rohit Prakash
  • 1,975
  • 1
  • 14
  • 24
  • 1
    Performance is [the weakest reason to avoid exceptions](http://yoda.arachsys.com/csharp/exceptions2.html). You should use them in exceptional circumstances only. – Tim Schmelter Jan 27 '15 at 12:05
  • 1
    `out Exception exception` that signature I have never seen before. – Magnus Jan 27 '15 at 12:06
  • @TimSchmelter, that is what I said above, "avoiding exception is more preferred for the performance". – Rohit Prakash Jan 27 '15 at 12:08
  • @Magnus, offcourse this pattern is not a standard coding style, but in case if he would like to know the reason of exception outside the method call, he could get it this way or either return exception instead of Boolean. – Rohit Prakash Jan 27 '15 at 12:10
  • @RohitPrakash: but my point was that performance is **not** the main reason to avoid exceptions. To quote J.Skeet: "If you ever get to the point where exceptions are significantly hurting your performance, you have problems in terms of your use of exceptions beyond just the performance." – Tim Schmelter Jan 27 '15 at 12:13
  • Thanks folks for the quick answer. I will go with the one without exceptions. I also heard that exceptions should be exceptional(pun intended), and if used relatively lot it could cause performance loss. I just needed confirmation. – Soni Jan 27 '15 at 12:22
  • Also, how do I mark somebody as the answer? I'm new to this :) – Soni Jan 27 '15 at 12:26
  • @Soni you click the little tick thing next to their answer so it goes green. – tom redfern Feb 06 '15 at 10:29