What is the most appropriate way to handle corrupt input data in a C# constructor?

Question

I'm reading data in from a file and creating objects based on this data. The data format is not under my control and is occasionally corrupt. What is the most appropriate way of handling these errors when constructing the objects in C#?

In other programming languages I have returned a null, but that does not appear to be an option with C#.

I've managed to figure out the following options, but I would appreciate advice from more experienced C# programmers:

Option 1. Read the file inside the constructor and throw an exception when the source data is corrupt:

try
{
    obj = Constructor(sourceFile);
    ... process object ...
}
catch (IOException ex)
{
    ...
}

Option 2. Create the object, then use a method to read data from the source file:

obj = Constructor();
obj.ReadData(sourceFile);
if (obj.IsValid)
{
    ... process object ...
}

or possibly throw exceptions on error:

obj = Constructor();
try
{
    obj.Read(sourceFile);
    ... process object ...
}
catch
{
    ...
}

Option 3. Create the object using a static TryParse method:

if (ObjClass.TryParse(sourceFile, out obj))
{
    ... process object ...
}

and if so, should I implement option 3 internally using option 1?

public static bool TryParse(FileStream sourceFile, out ObjClass obj)
{   
    try
    {
        obj = Constructor(sourceFile);
        return true;
    }
    catch (IOException ex)
        return false;
}

Answer 1

I would do something along the lines of option 3):

class ObjectClass
{
    protected ObjectClass(...constructor parameters your object depends on...)
    {
    }

    public static ObjectClass CreateFromFile(FileStream sourceFile)
    {
        .. parse source file
        if (parseOk)
        {
            return new ObjectClass(my, constructor, parameters);
        }
        return null;
    }
}

And then use it like this:

ObjClass.CreateFromFile(sourcefile);

In general the constructor should take as parameters all properties which essentially define the class. Doing heavyweight calculations (like parsing a file) is best left to factory methods as it is usually not expected for the constructor to perform complex and potentially long running tasks.

Update: As mentioned in comments a better pattern is this:

    public static ObjectClass CreateFromFile(FileStream sourceFile)
    {
        .. parse source file
        if (!parseOk)
        {
            throw new ParseException(parseErrorDescription);
        }
        return new ObjectClass(my, constructor, parameters);
    }

    public static bool TryCreateFromFile(FileStream sourceFile, out ObjectClass obj)
    {
        obj = null;
        .. parse source file
        if (!parseOk)
        {
            return false;
        }
        obj = new ObjectClass(my, constructor, parameters);
        return true;
    }

Answer 2

From Microsoft Constructor Design Guidelines (MSDN),

Do throw exceptions from instance constructors if appropriate.

Constructors should throw and handle exceptions like any method. Specifically, a constructor should not catch and hide any exceptions that it cannot handle.

Factory Method is not the right way to approach this problem. See Constructors vs Factory Methods

From Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries

5.3 Constructor Design

Consider using a static factory method instead of a constructor if the semantics of the desired operation do not map directly to the construction of a new instance, or if following the constructor design guidelines feels unnatural.

Do throw exceptions from instance constructors if appropriate.

.NET BCL implementations do throw exceptions from constructors

For example, the List Constructor (Int32), throws an ArgumentOutOfRangeException when the capacity argument of the list is negative.

var myList = new List<int>(-1); // throws ArgumentOutOfRangeException

Similarly, your constructor should throw an appropriate type of exception when it reads the file. For example, it could throw FileNotFoundException if the file does not exist at the specified location, etc.

More Information

• Code Contracts
• Throwing exceptions from constructor in .Net
• Throwing ArgumentNullException in constructor?
• Constructor parameter validation in C# - Best practices

Answer 3

Both Options #1 and #3 are good choices and common in the .Net framework. It's also common to provide both for the same type. Consider Int32.TryParse and Int32.Parse. Providing both gives developers a bit more flexibility without detracting from the integrity of the type.

I would strongly advise you to avoid #2. This pattern forces both the type author and type consumer to handle instances of the type in multiple states

• Constructed but not fully initialized
• Initialized and valid
• Initialized and invalid

This puts a burden on every consumer to deal with instances being in all different states (even if the response is to just throw). Additionally it forces a non-standard pattern on consumers. Developers have to understand your type is special and that it needs to be constructed and then initialized. It goes against the standard way objects are created in .Net.

Note for #3 though I would approach it a bit different. The exception form should be implemented in terms of the try form. This is the standard pattern when providing both options to the user. Consider the following pattern

class MyType { 
  struct ParsedData { 
    // Data from the file
  }

  public MyType(string filePath) : this(Parse(filePath)) { 
    // The Parse method will throw here if the data is invalid
  }

  private MyType(ParsedData data) {
    // Operate on the valid data.  This doesn't throw since the errors
    // have been rooted out already in TryParseFile
  }

  public static bool TryParse(string filePath, out MyType obj) {
    ParsedData data;
    if (!TryParseFile(filePath, out data)) {
      obj = null;
      return false;
    }

    obj = new MyType(data);
    return true;
  }

  private static ParsedData Parse(string filePath) {
    ParsedData data;
    if (!TryParseFile(filePath, out data)) {
      throw new Exception(...);
    }
    return data;
  }

  private static bool TryParseFile(string filePath, out ParsedData data) {
    // Parse the file and implement error detection logic here
  }
}

Answer 4

All these solutions work, but as you said, C# doesn't allow to return null from a constructor. You either get an object or an exception. Since this is the C# way to go, I wouldn't choose option 3, because that merely mimics that other language you're talking about.

Lots of people [edit] among which is Martin, as I read in his answer :) [/edit] think it is good to keep your constructor clean and small. I'm not so sure about that. If your object is of no use without that data, you could read in the data in the constructor too. If you want to construct the object, set some options, and then read the data (especially with the possility to try again if the read fails), a separate method would be fine as well. So option 2 is a good possibility too. Even better maybe, depending mainly on taste.

So as long as you don't choose 3, choose the one you're the most comfortable with. :)

Answer 5

I would not put anything into a constructor that might throw an exception - except for if something goes really wrong.
If your constructor has a possible return value other than a valid object, you should encapsulate it.

The safest way would probably be to create a factory method (public static function in the class that accepts a file reference and returns a new instance of the class or null). This method should first validate the file and its data and only then create a new object.

If the file data has a simple structure, you can first load it into some local variable and construct the object with this data. Otherwise, you can still decide - inside of your factory method - if you rather want to try / catch the construction or use any of the other points mentioned above.