views:

231

answers:

5

I have a number of methods doing next:

var result = command.ExecuteScalar() as Int32?;
if(result.HasValue)
{
   return result.Value;
}
else
{
   throw new Exception(); // just an example, in my code I throws my own one
}

I wish I could use operator ?? like this:

return command.ExecuteScalar() as Int32? ?? throw new Eception();

but it generates a compilation error.

Is it possible to rewrite my code or there is only one way to do that?

+13  A: 

You can't do that directly - the second operand of ?? needs to be an expression, not a throw statement.

There are a few alternatives if you're really just trying to find an option which is concise:

You could write:

public static T ThrowException<T>()
{
    throw new Exception(); // Could pass this in
}

And then:

return command.ExecuteScalar() as int? ?? ThrowException<int?>();

I really don't recommend that you do that though... it's pretty horrible and unidiomatic.

How about an extension method:

public static T ThrowIfNull(this T value)
{
    if (value == null)
    {
        throw new Exception(); // Use a better exception of course
    }
    return value;
}

Then:

return (command.ExecuteScalar() as int?).ThrowIfNull();

Yet another alternative (again an extension method):

public static T? CastOrThrow<T>(this object x) 
    where T : struct
{
    T? ret = x as T?;
    if (ret == null)
    {
        throw new Exception(); // Again, get a better exception
    }
    return ret;
}

Call with:

return command.ExecuteScalar().CastOrThrow<int>();

It's somewhat ugly because you can't specify int? as the type argument...

Jon Skeet
Any reason for the downvote?
Jon Skeet
I think it was because you didn't answer as Tony. Anyway, countered it for you. You're on the right track here, but Think there is a nicer, generalised, technique that I'll add as my own response(at the risk of a downvote)
Phil Nash
Jon, could you use generic parameter constraints to create two `CastOrThrow<T>` methods, one for value types/structs and one for reference types? The former would use `T?` whereas the latter would use `T`.
Adam Maras
@Adam: Unfortunately, you can't have two methods where the only difference to their signature is the output type and/or generic constraints.
LukeH
Extension method! Simply brilliant `ThrowIfNull` +1
Alex Bagnolini
+5  A: 

If you just want an exception when the returned value isn't an Int32 then do this:

return (int)command.ExecuteScalar();

If you want to throw your own custom exception then I'd probably do something like this instead:

int? result = command.ExecuteScalar() as int?;
if (result == null) throw new YourCustomException();
return result.Value;
LukeH
Yes, an exception will be thrown. But perhaps an invalid cast exception isn't the appropriate exception to be thrown; it's possible there needs to be an application-specific exception in the event that that command doesn't return a value.
Adam Maras
@Adam: I really don't think that merits a downvote! The example code in the question throws a plain `Exception`. An `InvalidCastException` or `NullReferenceException` is *more* appropriate and informative than a plain `Exception` with no extra details.
LukeH
Let me rephrase; it's possible that in the event that the command doesn't return a value, an application-specific exception (for example, a `NoRecordsExistException`) would need to be thrown (yes, I'm aware that the poster didn't mention such a thing, but some people do remove such specificities from their questions.) To do so, one would have to wrap the statement you posted in a `try`/`catch` block, which would defeat the purpose of condensing the code.
Adam Maras
@Adam: That's true, but without further information from the OP I would reiterate that my code is *simpler* than the code in the question and throws exceptions that are *more* informative/appropriate. If that's not what the OP requires then they should clarify in the question.
LukeH
I likes this. How about:try{ return (int)command.ExecuteScalar();}catch{ throw new NotFoundException();}
ssg
@Luke: true. I'm a patterns/practices geek, so I'm always looking for the "ideal solutions." I can't change my vote anymore... but you do provide a compelling argument.
Adam Maras
You *can* change your vote
Phil Nash
@ssg: I'm not overly keen on using `try...catch...throw` like that unless absolutely necessary. I've updated the question to show what I probably would do if a custom exception is needed.
LukeH
@Phil: odd. It let me now, it didn't earlier. I got an error about the vote being too old, and that I couldn't change it unless the answer was modified. Either way, I've changed it now.
Adam Maras
You could use Execute-Around to remap the exception (that would involve having another method who's purpose it was to call you back, wrapped in a try-catch block, throwing an exception of your choice. But if you're doing that you may as well use my Enforcements proposal anyway :-)
Phil Nash
Hello everybody. I edited my post. As Adam mentioned, I wrote Exception just as example. My original code throws my own FinanceResultNotFoundException :)
abatishchev
+2  A: 

You're not going to be able to throw an exception on the right side of the null coalescing operator. The reason behind this is that that the right side of the operator needs to be an expression, not a statement.

The null coalescing operator works like so: if the left value of the operator is null, return it; otherwise, return what's on the right of the operator. The throw keyword doesn't return a value; hence, it can't be used on the right side of the operator.

Adam Maras
+1  A: 

The reason you can't do:

return command.ExecuteScalar() as Int32? ?? throw new Exception();

Is because throwing an exception is a statement, not an expression.

If you're just looking to shorten the code a little bit, perhaps this:

var result = command.ExecuteScalar() as Int32?;
if(result.HasValue) return result;
throw new Exception();

No need for the else.

Josh Smeaton
This would only work if the function this return statement is in returns an Object. Any other return type would result in a compiler error, as the left and right expression types of the null coalescing operator are different types.
Adam Maras
I thought that, hence the 'possibly'. I removed that part of my answer.
Josh Smeaton
I prefer inverting the check for value, and throwing if it doesn't have a value. Sounds more logical.
Dykam
+5  A: 

As has been said, you can't do this with the ?? operator (well, not without some contortions that don't seem to fit with your aim of making this cleaner).

When I see this pattern emerging I immediately think of Enforcements. Originally from the C++ world they transfer to C# pretty well, although are arguably less important most of the time.

The idea is that you take something of the form:

if( condition )
{
  throw Exception;
}

and converts it to:

Enforce<Exception>( condition );

(you can further simplify by defaulting the exception type).

Taking it further you can write a set of Nunit-style methods for different condition checks, e.g.;

Enforce<Exception>.NotNull( obj );
Enforce<Exception>.Equal( actual, expected );
Enforce<Exception>.NotEqual( actual, expected );

etc.

Or, better still by providing an expectation lamba:

Enforce<Exception>( actual, expectation );

What's really neat is that, once you've done that, you can return the the actual param and enforce inline:

return Enforce( command.ExecuteScalar() as Int32?, (o) => o.HasValue ).Value;

... and this seems to be the closest to what you're after.

I've knocked up an implementation of this before. There's a couple of little niggles, like how you generically create an exception object that takes arguments - some choices there (I chose reflection at the time, but passing a factory in as an extra parameter may be even better). But in general it's all pretty straightforward and can really clean up a lot of code.

It's on my list of things to do to knock up an open source implementation.

Phil Nash
Any reason not to use an extension method here? "return (command.ExecuteScalar() as int?).Enforce(x => x.HasValue);" reads slightly better to me... although it may be worth changing the name at that point though. I do like the idea of using a predicate.
Jon Skeet
Mostly because when I first did this in C# I was using C#2 ;-) I didn't use the lambda for the predicate originally for the same reason, but that was a no brainer to move to. I think the extension method can be made to work nicely, but will have to play with that a bit.
Phil Nash