Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Parameter validation, or let it fail?

Tags:

c#

Aprroach 1:

public static void SendMail(string from, string to, string subject, string body)
{
   if(String.IsNullOrWhiteSpace(from))
      throw new ArgumentNullOrWhiteSpaceException("from");

   if(String.IsNullOrWhiteSpace(to))
      throw new ArgumentNullOrWhiteSpaceException("to");

   var msg = new MailMessage(from, to, subject, body) { IsBodyHtml = true };

   using(var smtp = new SmtpClient())
      smtp.Send(msg);
}

Approach 2:

public static void SendMail(string from, string to, string subject, string body)
{
   var msg = new MailMessage(from, to, subject, body) { IsBodyHtml = true };

   using(var smtp = new SmtpClient())
      smtp.Send(msg);
}

Why would I validate the parameters in approach 1, and not just wait for MailMessage to throw an exception (approach 2), telling me that I have passed a empty from or to value, to the constructor?

So why would I throw my own exception?

like image 310
Niels J. Avatar asked Sep 23 '13 07:09

Niels J.


3 Answers

The reason for this is quite simple - it makes it easier to debug.

For any given method (and this is truer of more complex methods) that requires a non-null parameter, it's going to be much easier for someone debugging an exception scenario to see an explicit exception from SendMail saying, "hey, 'from' is null; I need it not to be," than it is to have some method call within SendMail (or even some nested method call within that) throw a NullReferenceException (which, ultimately, is what will happen if none of the methods in question perform null checks).

Then, you have the scenario whereby - 6 months down the line - you decide that SendMail needs to do something else; e.g. (as a trivial example) set some kind of audit flag in a database. Now, if you just let the method fall over, you have an invalid flag (or you may do, depending on the order of things inside your method). Much better to say "actually, if my parameters are invalid, just fail immediately," rather than let the method carry on and have potential side effects.

like image 149
Ant P Avatar answered Nov 09 '22 09:11

Ant P


In general, I think that throwing your own exception is justified when you can give more relevant information than the function you're calling can (either to handle a more specifix exception in your code or to return a better error message to the user).

In this case it seems that you wouldn't add any information that Send() couldn't.

like image 44
Simon Avatar answered Nov 09 '22 09:11

Simon


smtp.SendMail will throw an InvalidOperationException (in the system namespace)

In this you are throwing a more suitable exception type, from the type type it easier to understand and catch the exception. InvalidOperationException is a very general class. By throwing your own exception the code is more readable and the same exception can be handled even if you change the method at a later point, for example by using another mailclient.

like image 32
Filip Avatar answered Nov 09 '22 07:11

Filip