How should I handle invalid user input in this case?
I am writing a plugin for a geometric modeling program and I have an abstract class based on a Curve object. A curve is considered fair if it is flat, closed and does not intersect itself.
Then I have a chain of other methods that reference this curve that do things like make a surface out of it, or extrude it into volume. These methods will throw an exception if they are called when BaseCurve is invalid. Right now my program just crashes if the user invalidates one of the curves and I am trying to figure out how best to manage my invalid entry by my program.
This is 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.
This is not unexpected as the underlying curve is invalid, as I know the user will create an invalid curve when using my program. As I understand it, an exception should usually only be thrown when the program reaches a point where something unexpected happens and it doesn't know how to proceed?
Should I instead return NULL from GetSurface () if the curve is not valid and then every GetSurface () based method also return null if GetSurface ()? This seems to be more difficult to debug. I know that I ended up forgetting to check if the return value is somewhere NULL and ended up with some ArgumentNullException that completely tracks AbsCurveBasedObject.GetSurface ()
So, would it be better to have if / else or try / catch blocks all over the tracing of the place to be processed when the user somehow deprives the base curve property of its value?
source to share
The Official C # Design Guidelines say to throw the exception and let the UI layer handle the exception.
source to share
Ideally, you never want to throw an exception if you can avoid it. However, this does not necessarily mean that returning zero is the right thing to do! Perhaps this is the wrong thing.
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 can make the caller like this:
Foo foo = GetFooFromUser();
try
{
blah = MakeMeABlah(foo);
}
catch(...)
{
// Tell user that input foo was invalid
}
It's not that good. The solution to your problem is to make IsValid open:
Foo foo = GetFooFromUser();
if (!IsValid(foo))
// Tell user that input foo was invalid
else
blah = MakeMeABlah(foo);
And hey, no exception handling, at the cost of having to call IsValid twice. If validation is cheap, who needs it? If validation is expensive, you may need to refactor this further to have an internal version of MakeMeABlah that does not recheck the release version.
source to share
Why do you need a curve to be plotted if it is not valid. By using the builder pattern, you can provide the ability to create only valid Curve objects and allow the frontend to handle invalid state there.
This is (I think) the gist of Eric's idea, to expose an external way of checking the state before you create an invalid object.
Note. After you only have valid Curve objects in your building, you can remove the dangerous if-create boolean "bool BaseCurveIsValid". :)
source to share