CODEWITHSUNDEEP
c#
Martin
Martin

Reputation: 40593

Should a method which can throw an exception has "throw" in its name?

I am writing a method which makes sure that the caller is authorized to execute it. When the user isn't authorized, I have to throw an exception (see code below). What should be the name of that method?

public void Compute() {
    this.CheckIfCallerAuthorizedOrThrow(); // Is it a good name? Better idea?

    // ...
}

I want to make sure that someone who reads the code understand that the flow of the method can be broken here by an exception. Is that a good or bad practice in this case to have a method name with "throw" in it?

Upvotes: 5

Views: 1932

Answers (5)

CPerkins
CPerkins

Reputation: 9018

This is obviously a personal taste issue, but a more common idiom is in the "assert" family, something like assertCallerIsAuthorized().

Upvotes: 1

Phil
Phil

Reputation: 6679

Honestly, a method name should describe exactly what it is supposed to do. So in a way, that's a good name. However, an exception is a way of saying "This method couldn't complete successfully and here's why...". Just about every method you write could throw an exception, so it wouldn't make sense to document exceptions in the name of every method.

You should consider using XML documentation. XML documentation is more specific too. It tells what type of exception is being thrown, as well as the specific reasons it will be thrown.

I do actually use method names with "throw" in them, but they are usually really short private methods that I reuse in several other public methods. They usually look like this:

private void doSomethingOrThrow(object args)
{
    if (args == null)
        throw new ArgumentNullException();
    doSomething();
}

So, in short, that's potentially a good method name. I just wouldn't use "throw" in every single method that throws exceptions. Besides, a good developer will need to look at the documentation anyway to see what kind of exceptions are being thrown (I certainly hope he's not just catching the generic Exception all the time...)

Upvotes: 8

Alexei Levenkov
Alexei Levenkov

Reputation: 100555

Any method can throw exceptions, so adding OrThrow/Throws does not necessary provide enough value to make name longer.

Try using assert/ensure/verify instead of "check" (which feels like it should true/false) in the name. I.e. see if it feels reasonable for you that next line after "EnsureCallerAuthorized" will only be executed by authorized user.

Upvotes: 2

Teoman Soygul
Teoman Soygul

Reputation: 25742

No it starts to look Java's checked exceptions that way. Rather document all thrown exceptions using XML documentation. For instance:

/// <summary>
/// Checks to see if the caller is authorized to access the resource.
/// </summary>
/// <exception cref="ArgumentException">
/// Throws invalid argument exception when an invalid parameter is supplied.
/// </exception>
CheckIfCallerAuthorized(sring myArgument)
{
  throw new ArgumentException("Invalid argument was provided for myArgument.");
}

Upvotes: 4

flq
flq

Reputation: 22859

From what I read the fact that this method cannot be executed is not exceptional. It can frequently happen throughout the course of program execution. If that assumption is correct, I would advice against actually using an exception to control program flow.

Upvotes: 0

Related Questions