Exceptions or something else?

Question

I'm in the process of creating a program that can model points, vectors, lines and segments. I have the classes built and confirmed to be working correctly. The classes are built to handle multiple dimensions, so as an example, the same point class is used for R1, R2, R3, ... Rn.

The issue that I am dealing with is how to handle operations between classes if the dimensions are different. For example, you can add two vectors that are in R2 but you cannot add a vector that is in R2 and another in R3.

My original plan was to create an exception class that I would throw if the dimensions between the two objects was different. This would work but would entail needing to wrap a lot of operations in try/catch blocks. Is this the best course of action to take or is there a better method that i could impliment?

Below is my point class:

public class Point
{
 private List<double> values;
 public List<double> VALUES
 {
 get { return values; }
 set { values = value; }
 }
 public int DIMENSION
 {
 get { return VALUES.Count; }
 }
 public Point(List<double> args)
 {
 values = new List<double>();
 this.VALUES = args;
 }
 public override string ToString()
 {
 string temp = "( ";
 for (int i = 0; i < DIMENSION; i++)
 {
 temp += VALUES[i];
 if(i != DIMENSION - 1)
 temp += ", "; 
 }
 temp += ");";
 return temp;
 }
 public static double Distance(Point point1, Point point2)
 {
 if (!SameDimension(point1, point2))
 {
 //add exception
 }
 double temp = 0;
 for (int i = 0; i < point1.DIMENSION; i++)
 {
 temp += Math.Pow(point2.VALUES[i] - point1.VALUES[i], 2);
 }
 return Math.Sqrt(temp);
 }
 public static Point Midpoint(Point point1, Point point2)
 {
 if (!SameDimension(point1, point2))
 {
 //add exception
 }
 List<double> temp = new List<double>();
 for (int i = 0; i < point1.DIMENSION; i++)
 {
 temp.Add((point2.VALUES[i] + point1.VALUES[i]) / 2);
 }
 return new Point(temp);
 }
 //This is the method used to check dimensions
 public static bool SameDimension(Point point1, Point point2)
 {
 if (point1.DIMENSION != point2.DIMENSION)
 return false;
 else
 return true;
 }
}//working, needs exceptions added

This is the most simple class but all check the dimension by using this meathod.

EDIT: The reason for needing to encase operations in try/catch blocks is say I have:

Point p1 = new Point(new List<double> { 0, 0});
Point p2 = new Point(new List<double> { 3, 3});
Point p3 = new Point(new List<double> { 4, 4, 4});
double distance = Point.Distance(p1, p2);

This would work completely fine, giving the distance of 3sqrt(3). But if this was tried:

distance = Point.Distance(p1,p3);

An error should occur because you are trying to compare a point in 2-Dimensions to a point in 3-Dimensions. Code will be written to reduce the risk of this happening, but I want a safe-guard in place in case this situation arises. If I go with the exception route, try/catch blocks will be needed. This is more apparent in the Vector class where when you add two vectors a new vector is returned. In that case, if a dimension error is encountered, the return value would be null, causing the possibility of other errors. Using exceptions, in my opinion, would mitigate potential errors as well as give me a capacity to track when they occur during run-time.

• Ben Aaronson: I'm not sure this is on topic for code review as this seems more like a work in progress from what you're describing.
• JRLambert: @BenAaronson Which SE site would you suggest be a better place? SO?
• Ben Aaronson: Possibly programmers, but I'm not familiar enough with the sites to recommend with any confidence. If you wait, somebody else will come along and either say it's fine here, tell you where it should be, or migrate it.
• Ben Aaronson: But until then... could you clarify why your approach "would entail needing to wrap a lot of operations in try/catch blocks"? Why would you want to catch these exceptions, and how would you handle them?
• JRLambert: @Mat'sMug Everything works as intended. I wrote test drivers for all the classes and fixed all the bugs.

Answer 1

Okay, I'll throw in a review. The part more directly addressing your question is at the end

Naming

• Usually in C# you will see PascalCase used for public members. For private members, it's a bit more varied, but _camelCase (note the prefixed underscore) is quite popular. What I've never seen before is ALLCAPS for public members. You can of course choose your own casing conventions for your project, but in general it's a positive to be consistent with the conventions of the language. It's helpful when working in a group or contributing to open-source, and it's also helpful to keep code you write consistent in appearance with code you call from other libraries (including the core .NET libraries)

• Casing aside, not all of your variables are well-named:

  1. args is not a good name for the variable you pass in to the Point constructor. You already have a name for these, values, so why not keep consistent?

  2. Having said that, Values isn't actually a great name either. A point doesn't have "values", it has "coordinates", so that might be a better name.

  3. temp inside your ToString method isn't ideal either. It's declared on the first line of the method, and returned at the end, that's about as not-temporary as a local variable can get! I generally use result here, though I know some people hate it.

  4. Likewise, temp inside your Distance and Midpoint methods can be renamed along the same principle of giving it a meaningful name which describes what the variable actually is.

Cleaning up code

A few bits and pieces that can be expressed more concisely and readably

• You don't need to declare a separate backing field for VALUES, C# has some nice syntactic sugar to do this for you:

  public List<double> VALUES { get; set; }
  

  This works exactly the same way as what you have, except that since the values field no longer exists, you will have to update the first line of your constructor to use VALUES instead.

• The SameDimension method can be much more concisely expressed as

  public static bool SameDimension(Point point1, Point point2)
  {
   return point1.DIMENSION == point2.DIMENSION;
  }
  

• You can also get rid of the loop (and the hard-to-name variable along with it) in ToString()

  public override string ToString()
  {
   return "( " + String.Join(", ", VALUES) + " )";
  }
  

Don't Repeat Yourself

Generally this adheres pretty well to the Don't Repeat Yourself (DRY) principle. One place where I see repeated logic is in the check for the exception you're describing in the question. This can be moved out to its own method:

private static void ValidateSameDimension(Point point1, Point point2)
{
 if(!SameDimension(point1, point2))
 throw new MismatchingDimensionsException(); // Or whatever you decide to call it
}

Then you can just call ValidateSameDimension at the top of methods which need that validation.

Using Exceptions

Your approach, using an exception in the case of trying to perform an action between points of different dimensions, is good. But what you've written about it is actually a little contradictory. So trying not to move too far outside the realm of a code review, here's some advice about how should use exceptions.

As a basic review of exceptions, when an exception is thrown from a method, that method immediately terminates and it is passed up to the next method in the call chain (the one that called it), and continues being passed up until it is caught.

So let's say I write a method which calls DoFoo(), and I know that DoFoo() can throw a particular exception. My options are more or less as follows:

• Surround DoFoo() with try...catch because I know how to fix (or attempt to fix) the problem that the exception represents, or because I know I can safely ignore the exception. An example of this is if DoFoo() involves a remote webservice call, and I want to retry the first time I get an exception from it, in case it was a temporary connectivity problem

• Surround DoFoo() with try...catch but re-throw it from within the catch. This is because I can't actually handle or ignore it, but there's something else I need to do before kicking it on up the call chain. Maybe I want to log it, or maybe there's some important clean-up I need to do inside a finally block.

• Surround DoFoo() with try...catch not because I can properly handle it, but because it's vital that my program doesn't actually terminate. This could be the case, for example, on a server-side process with an exception caused by a request sent by a client. It can't do anything more useful than log and display an error message to the client, but it needs to catch the exception so that it doesn't crash and stop processing requests altogether.

• Don't surround DoFoo() with try...catch. This is when I can't do anything more useful than pass it on up through the call chain, either to be handled higher up, or bubble up all the way to terminate the program with an error.

So, looking through those options, you'll probably see that in your case, generally the best one will be the last. This will actually have the opposite effect to what you were worried about. Not trying means that your Vector (for example) won't silently fail and return null, instead it will cease operating and the exception will keep bubbling on up. On the other hand, if you do surround it in a try block and leave the catch empty, that would silently gobble up the exception, and that's where you're at risk of getting weird, hard to debug behaviour.

• neontapir: I agree with @BenAaronson. Another option for the DoFoo(), try/catch logic would be to implement the Tester-Doer pattern of TryParse() fame.
• Ben Aaronson: @neontapir I would only suggest that if the exception is being thrown from code you have no control over. Otherwise, the TryDoWhatever should avoid using exception catching as part of its control flow. For example, in this case, TryGetMidpoint wouldn't be just try to call Midpoint and surround that with a try, it would check SameDimension, then call Midpoint without any try if it was true.
• neontapir: It looked to me like exceptions were being thrown for flow control in the OP code sample.

Answer 2

This would work but would entail needing to wrap a lot of operations in try/catch blocks. Is this the best course of action to take or is there a better method that i could impliment?

Take a look at Contracts class.

Here are screen shots of a video on code correctness

1. An otherwise trivial Sum(int x, int y) method with correctness checking using exceptions.
2. The same method using Contract class instead.

Before

public Int32 Sum(Int32 x, Int32 y)
{
 // Check input values
 if (x <0 || y <0)
 throw new ArgumentException();
 // Shorcut exit
 if (x == y)
 {
 // Perform the operation (optimization for 2x)
 var temp = x << 1;
 // Check output
 if (temp <0)
 throw new ArgumentException();
 return temp;
 }
 // Perform the operation
 var result = x + y;
 // Check output
 if (result <0)
 throw new ArgumentException();
} 

After

public Int32 Sum(Int32 x, Int32 y)
{
 // Check input values
 Contract.Requires<ArgumentException>(x >= 0 && y >= 0);
 Contract.Ensures(Contract.Result<Int32>() >= 0);
 // Perform the operation (bp & Ctrl-F11 to disassembly)
 if (x == y)
 return x << 1;
 return x + y;
}

Other Crazy Ideas

Use Tuple for strong typed dimentional-Points

The number of parameters in the Tuple.Create() method is actually part of the type.

public class Point<T> where T : Tuple<int,int,int> {
 public Point<T> (T 3Dimensions) {}
 public double Distance(T from ){}
}

• With this strong-type goodness I think we must have method implementations for every dimension-type --> no inheritance. But maybe Distance(T) for example would extract the Tuple items to build a int[] and pass that to a common method for actual calculation.

Use a Dimension-Point Factory

• We're trying to be kind to the Point clients giving them easy-peasy construction calls.

  public class DimensionPointFactory {
   public object Create(int[] thisType) { // "object" return type? ouch.
   object newPoint = null;
   switch(thisType.Length) {
   // .Net has 8-tuple built in. This is just one.
   case 3 :
   newPoint = new Point<Tuple<int,int,int>> (Tuple<int,int,int>.Create(thisType[0], thisType[1], thisType[2]);
   }
   return newPoint;
   }
  }