CODEWITHSUNDEEP
c#exception
AZ.
AZ.

Reputation: 7573

Do I throw exception explictly?

Say I have the following method:

void UpdateContentMethodA(Int32[] src, Int32[] dst)
{
    for(int i=0; i<src.Length; i++)
    {
        dst[i] = src[i];
    }
}

void UpdateContentMethodB(Int32[] src, Int32[] dst)
{
    if(src == null || dst == null) throw new System.ArgumentNullException();
    if(src.Length != dst.Length) throw new System.IndexOutOfRangeException();

    for(int i=0; i<src.Length; i++)
    {
        dst[i] = src[i];
    }
}

Do I write methodB or do you think methodA is fine?

EDIT: Sorry I was lazy about writing these methods:
these methods are just for demo purpose, the real code is using different data types and the exception instance is taking a helpful message in its constructor.
And I learned that good programmers should write good code even it's just for demo purpose. You guys are awesome, thanks!

Upvotes: 1

Views: 154

Answers (7)

Morten Mertner
Morten Mertner

Reputation: 9474

Given the above examples I'd go for method A, since your checks add little value. The general recommendation is to fail as early as possible though, and in that sense method B would be the best solution.

On the other hand, if you provide meaningful error messages to the exceptions (such as informing the caller whether src or dst is null, or what their lengths were), then they do add value and I would go for method B.

You may wish to take a look at Microsoft Code Contracts, which allows you to express both pre-conditions and post-conditions for methods.

Upvotes: 1

Daniel P. Bullington
Daniel P. Bullington

Reputation: 643

The correct implementation:

void UpdateContentMethodB(Int32[] src, Int32[] dst)
{
    if(src == null)
      throw new ArgumentNullException("src");

    if(src == null)
      throw new ArgumentNullException("dst");

    if(src.Length != dst.Length)
      throw new InvalidOperationException("Source length does not match destination length...blah blah blah");

    for(int i=0; i<src.Length; i++)
    {
        dst[i] = src[i];
    }
}

Upvotes: 1

Ryan Bennett
Ryan Bennett

Reputation: 3432

Method B if you expect that this is likely to happen and you require more information to debug. Method A if you feel these parameters are safe incoming to the method.

Upvotes: 0

SLaks
SLaks

Reputation: 887453

You should throw exceptions explicitly.

In addition, you should pass the parameter name to ArgumentNullException.

Upvotes: 1

Lasse V. Karlsen
Lasse V. Karlsen

Reputation: 391336

Personally I would change the code to this:

void UpdateContentMethodB(Int32[] src, Int32[] dst)
{
    if (src == null) throw new ArgumentNullException("src");
    if (dst == null) throw new ArgumentNullException("dst");
    if(src.Length != dst.Length) throw new ArgumentException("src and dst must be the same length", "src");

Upvotes: 9

JaredPar
JaredPar

Reputation: 754725

According to the .Net Guidelines for public facing methods you should be doing explicit argument validation and hence UpdateContentMethodB is the preferred method.

However in the second case I would not be using a IndexOutOfRangeException. Instead I would be throwing an ArgumentInvalidException.

Upvotes: 2

Will Dean
Will Dean

Reputation: 39500

It's generally considered better to throw explicit exceptions than to rely on being able to work out what happened from subsequent failures and stack examination.

However, you're not providing a very brilliantly written example. The ArgumentNullException should take the name of the argument which was null, and IndexOutOfRange is not really the right exception for "the two lengths must be the same but aren't."

I would throw probably throw ArgumentException for the length mismatch, and make sure I passed enough info into the construct so that it's clear what was going wrong.

Upvotes: 0

Related Questions