In one of my code reviews I saw this piece of code (those two methods are in the same class), IMO, and I believe that most of you will agree with me, this usage of exception is unnecessary.
private void CheckPrivileges()
{
// check user privileges
// for authorized user won't do anything.
// if not authorized:
throw new UserNotPrivException("user name", "action not allowed");
}
public void DoSomething()
{
try
{
CheckPrivileges();
// Continue your logic for authorized users.
}
catch (UserNotPrivException)
{
// showing message box which
// says that user has no permissions
ShowMessageBox ("User X has no privileges for this place.");
}
}
Why ?
- The Exception mechanism is slow
- A lot try&catch makes the code harder to read
- Why throw exception if you won't use it? and also throwing it between two methods in the same class?
I would have done it like this:
private bool IsUserPrivileged()
{
// check user privileges
// for authorized user - return true
// if not authorized - return false
}
public void DoSomething()
{
if (!IsUserPrivileged())
{
// showing message box which
// says that user has no permissions
ShowMessageBox("User X has no privileges for this place.");
return;
}
// Continue your logic for authorized users
}
If you want to check between those two (privileged or not) you can use the bool.
Moreover, when you are not really need the exception data.
Honestly,
I wrote this post before Tech-Ed, but didn't have enough time to publish it, in Tech-Ed, I talked with some other programmers that agreed with my opinion, but I still would like to hear your opinion,
what do you think about it?