views:

121

answers:

5

I have a method that reads data from a comma separated text file and constructs a list of entity objects, let's say customers.

So it reads for example,

Name
Age
Weight

Then I take these data objects and pass them to a business layer that saves them to a database. Now the data in this file might be invalid, so I'm trying to figure out the best error handling design. For example, the text file might have character data in the Age field.

Now my question is, should I throw an exception such as InvalidAgeException from the method reading the file data? And suppose there is length restriction on the Name field, so if the length is greater than max characters do I throw a NameTooLongException or just an InvalidNameException, or do I just accept it and wait until the business layer gets a hold of it and throw exceptions from there?

(If you can point me to a good resource that would be good too)

+3  A: 

I'd say - fail fast and loud. So throw an exception the moment there is an inconsistency (unless you want to get a hold of all the errors in the file and display it to the user... in which case you need a collecting parameter based design vs an exception based one).

  • Name custom exception classes to be as 'intention revealing' as possible. So a NameTooLongException is better than an InvalidNameException
  • Also the exception should be as helpful to the client to fix the problem .. (Ah I need to shorten the name in the file vs digging into the exception/code to know why is the Name invalid.) Shorten the discovery-and-resolution cycle time for your clients.
Gishu
It was tough to pick an answer! A lot of helpful answers but conceptually this was the most helpful, although I'm still handling some exceptions in the business layer
User
+1  A: 

If the data is unusable, it's probably best to throw the exception at the time you read the record. For example, if you know that character data in the age field could only be possible if the user messed with the file outside of your application. This is what would happen if you tried to serialize an object, messed with the serialized data, then tried to deserialize it.

If the data is intended to be manipulated outside of your application, or if you can recover some of the record or other records, then it's best to implement some kind of composite "errors" collection. You don't necessarily need to throw the exception. You could just create new Exception instances and put them in a collection.

In other words, an exception should only be thrown if you want to interrupt the current operation and you don't want to return partial results.

Josh Einstein
If you reading 10 entities from a file, and there is an error in number 5, should the whole operation fail or would you process 9 successfully and complain about the one. In this case the user can manually correct the files.
User
If you are able to use partial results, then I would definitely aggregate the errors into a collection and *not* interrupt the process by throwing.
Josh Einstein
+1  A: 

It depends how your logic is set up. If your file reading logic is generic (not a C# generic, a generic in the sense that it is not specific), it would be best to put the in the exception throwing a little higher up.

For example if this was the way things were set up:

// just an example
FileContents ReadFile(string path)
{
   // Don't necessarily throw exceptions related to data validity
}

SomeObject FromFile(string path)
{
   FileContents contents = ReadFile(path);
   // Do throw exceptions related to data validity

   // construct your object
}

It all comes down to what makes sense as to where to make the validation. As a general rule, you should only throw exceptions in exceptional situations (situations where you can not recover at that particular level). Some method higher up in the call stack may know what to do in the situation.

Also, if you must throw a lot of different types of exceptions, as a courtesy to the callers of your methods (indirect or direct, you or other developers) it would be a good idea to make your exceptions have a common base exception (other than System.Exception), so the can be caught with as few catch statements as necessary.

An example inheritance hierarchy might be:

  • System.Exception
    • DataLoadException
      • NameException
        • NameTooLongException
        • InvalidNameException
      • AgeException
      • ...

This way, if the caller only wants to know if the method succeed, they will only have to catch DataLoadException, instead of catching every type of throwable exception. If the caller instead wants to know exactly what went wrong, they can do that also.

Zach Johnson
A: 

It is best suited for the batch processing, I would say that, create a validation and ErrorData class. Data class will have Linenumber(CSV linenumber), Message and if you think any properties suitale.

Read the file and pass the entire collection into the validation class to validate and get the errors in collection of ErrorData class.

Process the correct data and Log or Throw the error. Here you are doing both processing the write data and also, informing the client (calling object) that there is error.

Somebody can open the log / or error message to correct the data.

JayachandranK
+1  A: 

It looks like you are doing a few things in your method:

  1. Read external datasource (file)
  2. Construct objects from read data.
  3. Pass objects to another part of the application ( to perform further processing).

I would limit validation to the following cases:

  1. If you can't read file throw here (no file, file is in wrong format).
  2. If you can't construct objects because of data throw here ("age" can't be parsed because of non-numerical characters).
  3. Validate Business Logic (name is too long or too small, weight is too big or too small) in Business Layer.
Andrei Taptunov
I think I agree with you. Especially point 3. I was concerned about validating business logic in the method that reads from the file because it seems to repeat validation logic in two places.
User
On the flip side, the situation is analogous to a UI scenario, where you do some validation in the UI and then additional more complex validation in the business layer.
User