What’s your opinion on a Throw() method?

Question

Lately I seem to run into a situation very frequently where I want to write something like the following:

var x =
    condition1 ? value1 :
    condition2 ? value2 :
    condition3 ? value3 :
    condition4 ? value4 :
    throw new InvalidOperationException();

Suppose the conditions are not simply constants so I can’t use switch, but they are not complex enough expressions to warrant saying that ?: is too unreadable.

Of course the above construct doesn’t compile because throw is a statement, not an expression. Therefore, I considered writing a method that does exactly that:

/// <summary>Throws the specified exception.</summary>
/// <typeparam name="TResult">The type to return.</typeparam>
/// <param name="exception">The exception to throw.</param>
/// <returns>This method never returns a value. It always throws.</returns>
public static TResult Throw<TResult>(Exception exception)
{
    throw exception;
}

[...]

var x =
    condition1 ? value1 :
    condition2 ? value2 :
    condition3 ? value3 :
    condition4 ? value4 :
    Ut.Throw<MyType>(new InvalidOperationException());

Are there any problems with this approach that I’m not seeing?

Answer 1

If it makes you feel any better, mscorlib and System assemblies both have internal ThrowHelper classes full of nothing but methods whose only statement is throw, like this:

internal static void ThrowKeyNotFoundException()
{
    throw new KeyNotFoundException();
}

Now I'm not going to say it's a good idea, but surely MS has a good reason for this, right?

Answer 2

I would avoid using such statement. It doesn't seem to be a right place for this construction inside ?: operators and I do not see major benefits in using it. I would consider using following options:

1)

Type variable;
if (condition1) variable = value1;
else if (condition2) variable = value2;
...
else throw new InvalidOperationException();

This has a disadvantage that in each line you have variable = but anyway compiler will let you know if you haven't initialized variable value before using it so it won't bother me much.

2) Also if your value1, value2, etc cannot be null then I would consider using this:

var variable = 
    condition1 ? value1 :
    condition2 ? value2 :
    condition3 ? value3 :
    condition4 ? value4 :
    null;
if (variable == null) throw new InvalidOperationException();

P.S.: Of course I cannot be sure without knowing the context but if you have to write such statements frequently then it already seems to be wrong for me, maybe something may be changed on higher level?

Answer 3

One problem with this approach is stack trace population.

The stack trace (generally) gives most immediate access to information relating to where whatever it was that went wrong actually went wrong. In this case it is populated with information spelling out extra method calls that are once removed from the actual location of the issue and may require (in absence of useful error messages, or their brain) developers (or, eeek! users) to think.

The fact that the method is generic aids in saturating the stack trace with irrelevant information.

Answer 4

Although you do solve a bit of the code clutter by using ternary operators instead of a bunch of if else statements, I believe you didn't solve the original problem.

Usually a long list of if else statements indicate the need to refactor. Usually this is possible by applying the Strategy pattern, where an 'algorithm' is selected at runtime. Whether or not this is a viable approach for your code is dependant on the specific situations where you use it.

If the conditions you are checking on can easily be attributed to different states or concrete implementations, I advise a redesign.

If there is no 'logical' way to separate the different assignments, I'd prefer Snowbear's solution with the null check. I do see the benefit of your solution however, and find it clear enough.

Answer 5

I think the only thing that's really wrong with this is that it's a bit unusual. That's not very wrong IMO, but may be hard to get past conservative reviewers. It's easier to accept this construct in the context of large expressions that aren't trivially expandable into multiple statements.

The only reason I can think of against such a feature in the core language is that it may be considered as not making the cost/benefit bar, along with lots of other small syntactic sugar ideas which look nice one by one, but together could make a language too complicated if not carefully considered and balanced.

Here's an example of something that becomes less elegant without Throw:

mylist.Add(new Item
{
    Prop1 = int.Parse(input1),
    Prop2 = input2 == "stuff" ? true : input2 == "notstuff" ? false : Ut.Throw(),
});

Just imagine this with a few more properties with more meaningful names.