debugcsharpMinor
Exceptions or something else?
Viewed 0 times
exceptionssomethingelse
Problem
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:
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 { 0, 0});
Point p2 = new Point(new List { 3, 3});
Point p3
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 values;
public List VALUES
{
get { return values; }
set { values = value; }
}
public int DIMENSION
{
get { return VALUES.Count; }
}
public Point(List args)
{
values = new List();
this.VALUES = args;
}
public override string ToString()
{
string temp = "( ";
for (int i = 0; i temp = new List();
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 addedThis 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 { 0, 0});
Point p2 = new Point(new List { 3, 3});
Point p3
Solution
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:
-
-
Having said that,
-
-
Likewise,
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
This works exactly the same way as what you have, except that since the
-
The
-
You can also get rid of the loop (and the hard-to-name variable along with it) in
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:
Then you can just call
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
-
Surround
-
Surround
-
Surround
-
Don't surround
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:
-
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?-
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. -
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.-
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 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 aspublic 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 thCode Snippets
public List<double> VALUES { get; set; }public static bool SameDimension(Point point1, Point point2)
{
return point1.DIMENSION == point2.DIMENSION;
}public override string ToString()
{
return "( " + String.Join(", ", VALUES) + " )";
}private static void ValidateSameDimension(Point point1, Point point2)
{
if(!SameDimension(point1, point2))
throw new MismatchingDimensionsException(); // Or whatever you decide to call it
}Context
StackExchange Code Review Q#49585, answer score: 7
Revisions (0)
No revisions yet.