Custom Exception

May 23, 2011 at 4:37 PM

Hi guys, we are trying out your tool which I found really useful but I believe there are still a lot of features missing.

For example, in our libraries we throw custom exceptions everywhere so even for a NullReferenceException I want to throw also
my custom exception.

Like this:

Conditions
   .Requires(myVariable)
   .IsNotNull<MyCustomException>("The variable can't be null");

Is there any chance you will introduce something like that or is there already a way to throw custom exceptions?

Thanks in advance. 

Coordinator
May 23, 2011 at 6:31 PM

Hi raffaeu,

The current release does not have a feature that allows you to throw custom exceptions. As a matter of fact, the beta's of Conditions contained a feature that allowed this, but I had to remove it from the stable release. You can read here why I had to remove it.

The syntax you propose is unfortunately not possible, due to the way C# type inference works. I had the discussion before with other developers and currently thinking about adding an OnFailure<T> method to the game. This would allow us to write the following:

    Condition.Requires(value)
       .OnFailure<InvalidOperationException>()
       .IsNotNull();

With the implementation I have in mind it will only be possible to call the OnFailure<T> method once per call to Requires.

Please let me know if such design solves your problem.

May 23, 2011 at 6:42 PM

I prefer Condition.Requires(value).ByThrowing<InvalidOperationException>().IsNotNull();

It seems to flow better for me... reading "Condition requires "value", by throwing invalid operation exception, is not null." Still not a great structure, but better than the OnFailure<T> signature in my opinion.

Of course I would prefer raffaeu's syntax as well, but as you say, it's not possible with C# type inference.

I hadn't considered limiting the ByThrowing<T> method call to once per Requires, but it makes sense.

May 23, 2011 at 6:58 PM

Given this:

"With the implementation I have in mind it will only be possible to call the OnFailure<T> method once per call to Requires."

 

I would, personally, put it directly in the Requires method.  Personally, the main use case for custom exceptions that I have require more elaborate initialization, in which none of these cases would really solve those issues.  Realistically, an "IsNotNull" criteria should throw ArgumentNullException, and not a custom exception, unless there is more at play here - and when there's more going on, you tend to need more elaborate initialization for the custom exception to make sense.

 

I'd rather see something like:

 

Condition.Requires(value, v => new MyCustomException(v, "Foo")).IsNotNull();

Granted, it doesn't read as nicely, but it gives you the ability to enforce (very cleanly) the "use once per method call" restriction, and also allows you to initialize your custom exception with a lot more flexibility than otherwise possible.  Given that this is the edge case, and not the most common usage, I think that's a very nice tradeoff.

This would turn Requires into having 6 potential overloads (3 with and without argument name of):

Requires<T>(T value) {...}
Requires<T>(T value, Func<Exception> exceptionGenerationDelegate) 
{
    // Can just call:
    this.Requires(value, v => exceptionGenerationDelegate());
}


Requires<T>(T value, Func<T, Exception> exceptionGenerationDelegate) { ... }

May 23, 2011 at 7:40 PM

I would also go with the Requires overloads too instead of adding a method that can be only called once. Once you get used to it, it seems more clear than the OnFailure method (and it's an elegant way of handling exceptions with custom initialization).

May 23, 2011 at 9:00 PM

For me it personally doesn't matter where to put the overload.

I believe that in order to respect the fluent language of Conditions you should have:

Condition.Requires(value)
 .OnFailure<InvalidOperationException>()
 .IsNotNull();

That would be awesome and it will fix definitely my problem.

Thanks, appreciated!

Coordinator
May 23, 2011 at 10:31 PM

Although I see the flexibility of an overload that takes an Func<Expression> delegate, as Reed proposes, it doesn't read well and will probably dazzle most developers who try to use Conditions. Especially the Func<T, Expression> delegate is, as Reed calls it, an edge case. When we use the extension method approach, we could of course add an overload that takes such delegate. For instance: Requires(value).OnFailure(() => new InvalidOperationException()).IsNotNull().

On the other hand, an OnFailure<T>, Otherwise<T>, ByThrowing<T> method (or whatever we should call it) has some tricky behavior. Although the T can be restricted to type Exception easily, we can't restrict it to be an exception that contains a public constructor that takes a string. Conditions should however be able to inject a descriptive error message into the exception. The Otherwise<T> method that was in the beta's actually did check on this (in the cctor) and the call would always fail when the exception didn't have a ctor(string). I don't think this will be a problem in common scenario's. Besides, according to the Framework Design Guidelines, exceptions should always have a public constructor that takes a string. Some calls however, could surprisingly fail. For instance, Requires(value).OnFailure<SqlException>() will fail, because for some strange reason System.Data.SqlClient.SqlException does have a public ctor(string). In fact, it is the only BCL exception that doesn't have such constructor. In that case perhaps users could fallback to using the overload that takes a Func<Exception> or Func<T, Exception.

btw. Restricting a method to only be callable directly after the Requires() method is easy to achieve. The trick is to change the contract of Requires() and let it return a sub type of ConditionValidator<T> (a non-breaking change), for instance a RequiresValidator<T>. The Otherwise<T> method can than be implemented as instance method on RequiresValidator<T> or as an extension method on RequiresValidator<T>. The Otherwise<T> can than simply return an ConditionValidator<T> and that will prevent the method from showing up again.

RBellamy suggest an ByThrowing<T> method. The problem I have with this name is that it suggests that this method will always throw. That's why I suggested OnFailure<T>. Although that name does set the expectations right, it still doesn't read as nice. I rather have something like Throws<InvalidOperation>OnFailure(), but this of course does not compile. Perhaps any of you has another alternative?

What do you think about this?

May 23, 2011 at 10:46 PM
Edited May 24, 2011 at 6:35 PM

I do agree that you can have the overload in the OnFailure/Otherwise/etc method - I was just suggesting that the extra method might be a bit much in terms of polluting the API for an edge case...

I had another thought with this - instead of chaining on Requires, you could also do something like:

   Condition.IfInvalidThrow<InvalidOperationException>().Requires(value).IsNotNull();

(with the potential for an Action overload which wouldn't require exceptions, or a Func<Exception>, etc...)

   Condition.OnInvalid(() => throw new InvalidOperationException("Foo")).Requires(value).IsNotNull();

 

This could also easily work in the same way for Ensures, and read the same way, which is nice for consistency.

Not sure if you like the readability more or less, but that would be fairly straightforward to implement as well. I do somewhat like the idea of having this before the Requires/Ensures, instead of afterwards, as it (at least to me) makes it a bit more clear when I read it that this is a different form of error handling than the standard.

May 23, 2011 at 10:48 PM

FYI - I agree with the arguments against ByThrowing - it reads as it's going to throw, not that it will throw if the condition fails or is invalid.

Jun 13, 2011 at 9:30 PM

Hi guys, any update on this?

Coordinator
Jun 14, 2011 at 1:19 PM

I'm currently thinking of implementing the feature as static method directly on the Condition class, as Reed suggests. I'm however still in doubt about the name of the method. The best I came up with was:

Condition.WithCustomExceptionOnFailure<InvalidOperationException>()
    .Requires(value).IsNotNull();
Please tell me what you guys think of it. Any other suggestions?

Jun 14, 2011 at 1:55 PM
The signature you mention looks pretty verbose by it's clear. I mean you can't misunderstand the meaning of such a kind of signature, it is clear. You may also consider to change it to a less verbose:

Condition.WithException<InvalidOperationException>()
    .Requires(value).IsNotNull();

Raffaeu
blog: http://blog.raffaeu.com

Coordinator
Jun 14, 2011 at 4:14 PM
Edited Jun 14, 2011 at 7:56 PM

I know it is verbose. That's the main reason why I'm hesitating in building it. With the normal indenting rules it has a width of almost 80 characters, which is a lot. On the other hand it is clear; very clear. WithException is less verbose, but less clear.

Has anyone have any other ideas?

Jun 15, 2011 at 12:44 AM

Personally, I don't mind verbose .. especially when it makes something clear. very clear.

(and i'll update the nuget package, naturally .. unless you wish to take proper ownership of it)

Coordinator
Jun 15, 2011 at 12:39 PM

Perhaps we can remove the 'Custom' part:

Condition.WithExceptionOnFailure<InvalidOperationException>()
    .Requires(value).IsNotNull();

Or is that less clear?

Purekrome: Eventually, I'd like to take ownership over the Nuget package, but it would be nice if you could publish the coming release.


Thanks

Jun 15, 2011 at 6:05 PM

Personally, I have no problem with a very long name.  Using WithCustomExceptionOnFailure seems perfectly reasonable to me.  However, WithExceptionOnFailure is "clear enough" as well.  

I do have a couple of comments on this naming...  Ithink that using Exception in the name is a bit much, though - you end up with FooExceptionBar<BazException>, since you'll always have to specify the exception type, and exceptions by convention always derive from Exception and include Exception in their name.

 

The current methods (ie: Requires) uses a fairly active verb.  I probably personally would prefer something more active like:

 

Condition.ThrowOnViolation<InvalidOperationException>().Requires(value).IsNotNull();

Coordinator
Jun 15, 2011 at 7:49 PM

We could perhaps even go with:

Condition.With<InvalidOperationException>()
    .Requires(value).IsNotNull();
Love to see your comments.

Jun 15, 2011 at 7:54 PM

At that point, I'd just go with:

Condition<InvalidOperationException>.Requires(value).IsNotNull();

That being said, I find this a bit less clear.  I don't mind extra typing in order to get clarity, especially with modern IDEs and auto-completion.

Coordinator
Jun 22, 2011 at 9:15 PM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Coordinator
Jun 22, 2011 at 9:18 PM

I'm currently adding the feature to the library. The syntax will be as follows:

Condition.WithExceptionOnFailure<InvalidOperationException>().Requires(value)
    .IsNotNull();

Jun 23, 2011 at 12:57 PM

Awesome, thank you!