How should I handle invalid user input in this case?

Question

I'm writing a plug-in for a geometric modeling program, and I have an abstract class that is based off a Curve object. The curve is only considered valid if it is planar, closed, and does not intersect itself.

I then have a chain of other methods that reference this curve that do things like make a surface out of it, or extrude it into a volume. These methods will throw an exception if they are called when the BaseCurve is invalid. Right now my program just crashes if the user invalidates one of the curves, and I'm trying to figure out the best way to have my program handle this invalid input.

Here's what my class looks like:

public abstract class AbsCurveBasedObject
{
    public abstract Curve BaseCurve
    {
        get;
    }

    public bool BaseCurveIsValid
    {
        get
        {
            Curve c = this.BaseCurve;
            ...
            //checks that curve is valid
            ...
            return true/false;
        }
    }

    public Surface GetSurface()
    {
         Curve c = this.BaseCurve();
         ...
         //magic that converts c to a surface
         //exception is thrown if c is invalid
         ...
         return surface;
    }

    public Surface GetVolume()
    {
         Surface s = this.GetSurface();
         ...
         //magic that converts s into a volume
         ...
         return volume;
    }
}

I'm not sure if GetSurface() should return NULL if the curve is invalid or if I should throw an exception.

It's not unexpected for the base curve to be invalid as I know the user will end up creating an invalid curve while using my program. It was my understanding that exceptions should usually only be thrown when the program reaches a point where something unexpected happens and it doesn't know how to continue?

Should I instead just return NULL from GetSurface() if the curve is invalid and then have every method that is based off of GetSurface() also return null if GetSurface() does? This seems like it will be harder to debug. I know I'll end up forgetting to check if a return value somewhere is NULL and end up with some ArgumentNullException that tracks all the way back to AbsCurveBasedObject.GetSurface()

So would it better to have if/else or try/catch blocks all over the place tracking to handle when the user some how invalidates the base curve property?

Answer 1

The official C# Design guidelines say to throw an exception and let the user interface layer handle the exception.

Answer 2

Ideally you never want to throw an exception if you can possibly avoid it. However, that does not necessarily mean that returning null is the right thing to do! That is probably the wrong thing to do.

Look at it this way. Your code probably looks like this:

Blah MakeMeABlah(Foo foo)
{
  if (!IsValid(foo)) throw new InvalidArgumentException("foo");
  // [make a blah from a foo]
}

You could make the caller look like this:

Foo foo = GetFooFromUser();
try
{
    blah = MakeMeABlah(foo);
}
catch(...)
{
   // Tell user that input foo was invalid
}

That's not so good. The solution to your problem is to make IsValid into a public method:

Foo foo = GetFooFromUser();
if (!IsValid(foo))
   // Tell user that input foo was invalid
else
   blah =  MakeMeABlah(foo);

And hey, no exception handling, at the price of having to call IsValid twice. If the validation is cheap, who cares? If the validation is expensive, then you might want to refactor this further to have an internal version of MakeMeABlah that doesn't re-do the check in the release version.

Answer 3

Why would want the Curve to be constructed if it's invalid. Using the builder pattern you can expose the ability to build only valid Curve objects and allow the UI to handle the invalid state there.

This is (I think) the essence of Eric's suggestion, expose an external way to validate state before you construct an invalid object.

Note: Once your building only valid Curve objects you can remove the dreaded if-creating boolean "bool BaseCurveIsValid". :)