1

I would like to ask if I'm getting a right syntax in using the try and catch in asp.net

My code goes like this:

public ActionResult Add_RefPerson(rms_referred_person ref_person)
    {
        if (ModelState.IsValid) {
            try
            {
                ref_person.rf_referreddate = Date();
                ref_person.rf_createdby = getBadge();
                ref_person.rf_updatedby = null;
                ref_person.rf_updateddate = null;
                ref_person.rf_isactive = true;
                db.rms_referred_person.Add(ref_person);
                db.SaveChanges();
                return RedirectToAction("Index");
            }
            catch (Exception ex) {
                throw ex;
            }
        }
        return Content("<script type='text/javascript'>alert('Cannot be saved');</script>");
    }

Is my try and catch in the right direction? or should I use this one.

public ActionResult Add_RefPerson(rms_referred_person ref_person)
    {

            try
            {
                   if (ModelState.IsValid) {
                         ref_person.rf_referreddate = Date();
                         ref_person.rf_createdby = getBadge();
                         ref_person.rf_updatedby = null;
                         ref_person.rf_updateddate = null;
                         ref_person.rf_isactive = true;
                        db.rms_referred_person.Add(ref_person);
                        db.SaveChanges();
                        return RedirectToAction("Index");
                }
            }
            catch (Exception ex) {
                throw ex;
            }

        return Content("<script type='text/javascript'>alert('Cannot be saved');</script>");
    }

Thanks a lot.

Isay
  • 95
  • 2
  • 16
  • 2
    use `throw;`., `throw ex;` obliterates the stack trace from the exception. – jdphenix Sep 11 '15 at 03:05
  • 2
    Please don't ever do `catch (Exception ex)` let alone do `catch (Exception ex) { throw ex; }`. The former is a bad practice and the latter is just terrible. You should only ever handle an exception that is truly exceptional **and** that you can recover from. Otherwise let the exceptions bubble to the top. Your code will have less bugs and be more maintainable if you do. – Enigmativity Sep 11 '15 at 03:06
  • Have a look to get more light on [Try Catch](http://stackoverflow.com/a/32394690/3796048) – Mohit S Sep 11 '15 at 03:20

3 Answers3

3

That is the correct syntax to catch all exceptions; however it is a pretty bad antipattern. This catches ex and immediately throws it again, clobbering the entire stack trace. If rethrowing is desired, write throw;

In this case you do not want to throw at all, so empty catch might be correct. Consider returning a bit more information about what went wrong, which would require placing the error return in the catch clause itself.

Joshua
  • 40,822
  • 8
  • 72
  • 132
1

The only difference is that the latter one will catch any exceptions in the

if (ModelState.IsValid)

line. Unless you think that line might actually throw an exception, they're identical.


You might also want to think more about casting such a wide net for exceptions (i.e., narrow down what actual exceptions you want to handle). The idea is to handle what you can and let everything else through for the upper layers to handle.

In addition, rethrowing an exception is best done with throw on its own rather than throw ex. The former preserves information that the latter will lose.

However, since you're not actually doing anything with the exception other than passing it up the tree, there's little point in catching it in the first place. Just execute the commands, if you get an exception, a higher level exception handler should take care of it without you messing around with try..catch.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
1

2nd option is safer as it covers ModelState check too. Also, throw ex; is not a great idea. you will not get complete stack trace. use throw;

         try
            {
                   if (ModelState.IsValid) {
                         ref_person.rf_referreddate = Date();
                         ref_person.rf_createdby = getBadge();
                         ref_person.rf_updatedby = null;
                         ref_person.rf_updateddate = null;
                         ref_person.rf_isactive = true;
                        db.rms_referred_person.Add(ref_person);
                        db.SaveChanges();
                        return RedirectToAction("Index");
                }
            }
            catch (Exception ex) {
                throw ex;
            }
Sateesh Pagolu
  • 9,282
  • 2
  • 30
  • 48