views:

127

answers:

4

I have a system where the employeeId must alway exist unless there is some underlying problem.

The way I see it, is that I have two choices to check this code:

1:

public void GetEmployee(Employee employee)  
{  
   bool exists = EmployeeRepository.VerifyIdExists(Employee.Id);  
   if (!exists)   
   {   
     throw new Exception("Id does not exist);  
   }  
}    

or 2:

public void GetEmployee(Employee employee)  
{  
  EmployeeRepository.AssertIfNotFound(Employee.Id);  
}  

Is option #2 acceptable in the C# language?

I like it because it's tidy in that i don't like looking at "throw new Exception("bla bla bla") type messages outsite the class scope.

+1  A: 

It depends what you mean by Assert.

You could use Debug.Assert (or Trace.Assert if you want it to also work in release mode). However this is not that useful because it halts the program and pops up a dialog box until a user presses something. This isn't so good for an unmonitored system. So I'd recommend throwing instead in most cases though as you can decide how you want to react to the error - stop the program, or just log and try to continue.

But if we assume that your Assert method checks its argument and possibly throws an exception, then yes I think that's a good way of doing it.

In fact to pick an example, in Jon Skeet's morelinq both methods are used. For example here:

public static IEnumerable<TSource> AssertCount<TSource>(
    this IEnumerable<TSource> source, 
    int count,
    Func<int, int, Exception> errorSelector)
{
    source.ThrowIfNull("source");
    if (count < 0) throw new ArgumentException(null, "count");
    errorSelector.ThrowIfNull("errorSelector");

    return AssertCountImpl(source, count, errorSelector);
}
Mark Byers
Yes. My Assert is for "Throwing an Exception". Thanks j.
guazz
@guazz: The first time I read your question I thought you were comparing throwing with asserting - and from looking at the other answers and votes, so did everyone else. You might want to reword your question to make it more clear.
Mark Byers
@Mark extension method for argument exception?
Chris Marisic
@Chris Marisic: Yes, ThrowIfNull is an extension method. It is implemented in this file: http://code.google.com/p/morelinq/source/browse/tags/1.0-beta/MoreLinq/ThrowHelper.cs
Mark Byers
A: 

Use exceptions, its what they are there for - exceptional circumstances. All the standard .NET libraries use this method of handling such circumstances so takes your cues from Microsoft.

Chris
+2  A: 

As a rule, you should only throw exceptions in exceptional circumstances. Since this one such circumstance, throwing an exception is the right thing to do.

Oded
A: 

The idea behind assertions, as I've always used them, are that they are instant feedback when running a debug build. Kind of an in your face that something happened. Or logged to file if the app is setup that way.

Exceptions are used to handle exceptional behavior, as noted above.

What I do, especially early in projects life cycle might be something like:

public void GetEmployee(Employee employee)  
{  
   bool exists = EmployeeRepository.VerifyIdExists(Employee.Id);  
   Debug.Assert( exists, "employee does not exist for id: " + Employee.Id );
   if (!exists)   
   {   
     throw new Exception("Id does not exist);  
   }  
}   

Perhaps refractoring out the Debug.Assert once the initial hiccups are dealt with.

DevSolo