OneTrueError - Automated exception handling

COM+ done right (refactoring a legacy application)

I’ve been spending a lot of time lately looking into a legacy application which uses COM+, i.e. got classes that inherits ServicedComponent. In this article I’ll show you some common mistakes and how you can correct them.

In that application a typical method looks like this:

public string SomeFunction(bool someVar)
{
	string s = null;
	DataSet ds = null;
	SomeReaderComponent reader = null;
	SomeWriterComponent writer = null;
	long trackNumber = 0;
	string method = "SomeFunction";
	try 
	{
		reader = new SomeReaderComponent();
		writer = new SomeWriterComponent();


		ds = reader.SomeInfo;
		trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["aktfno"]);

		if (trackNumber > Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["maxfno"])) {
			trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["minfno"]);
		}

		if (someVar == true) {
			s = "4512";
		} else {
			s = "9682";
		}

		s = s + Convert.ToString(trackNumber);

		writer.UppdateraFNO(trackNumber + 1);
		
		ContextUtil.SetComplete();
		return s + GetCheckSum(s);

	} 
	catch (SomeAppException ex) 
	{
		ContextUtil.SetAbort();
		HandleError(ex, method);
		throw ex;

	} 
	catch (Exception ex) 
	{
		ContextUtil.SetAbort();
		HandleError(ex, method);
		throw ex;
	} 
	finally 
	{
		//Clean up
		if ((reader != null))
			reader.Dispose();
		if ((writer != null))
			writer.Dispose();
		if ((ds != null))
			ds.Dispose();
	}
}

That’s all wrong (do note that the application was created in .NET 1.1, practices have changed).

Here’s why:

Exceptions

Exceptions are used to indicate that something went exceptionally wrong. That is, there is no way that the application can continue as promised. Therefore, it’s unlikely that a method can deliver the promised result by catching an exception.

In this case all catch blocks do only log and modify the exception a bit.

Exceptions the right way

First let’s remove all exceptions that doesn’t make the methods deliver the result as promised by the contract, which means all catch blocks.

In this application all COM+ components are always invoked through an asmx web service. I’m therefore writing a soap handler which will log the exception including all incoming message parameters.

That makes the exception logging/handling 100% transparent from the rest of the code. Hence we now got the code:

public string SomeFunction(bool someVar)
{
	string s = null;
	DataSet ds = null;
	SomeReaderComponent reader = null;
	SomeWriterComponent writer = null;
	long trackNumber = 0;
	string method = "SomeFunction";
	try 
	{
		reader = new SomeReaderComponent();
		writer = new SomeWriterComponent();


		ds = reader.SomeInfo;
		trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["aktfno"]);

		if (trackNumber > Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["maxfno"])) {
			trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["minfno"]);
		}

		if (someVar == true) {
			s = "4512";
		} else {
			s = "9682";
		}

		s = s + Convert.ToString(trackNumber);

		writer.UppdateraFNO(trackNumber + 1);
		
		ContextUtil.SetComplete();
		return s + GetCheckSum(s);

	}
	finally 
	{
		//Clean up
		if ((reader != null))
			reader.Dispose();
		if ((writer != null))
			writer.Dispose();
		if ((ds != null))
			ds.Dispose();
	}
}

Disposal

Disposing components is important when dealing with COM+ components. So it’s therefore correct of them to invoke dispose. There are however some problems.

Disposal the right way

The Dispose() method on datasets have no affect what so ever. Hence we can stop disposing them.

All COM+ components should however be disposed, but using try/finally is a bit cumbersome. Let’s use an alternative. In later versions of VB.NET and C# the using keyword was introduced. It basically calls Dispose() no matter if an exception was thrown or not. The above code could therefore be rewritten as:

public string SomeFunction(bool someVar)
{
	string s = null;
	DataSet ds = null;
	long trackNumber = 0;

	using (reader = new SomeReaderComponent())
	{
		using (writer = new SomeWriterComponent())
		{
			ds = reader.SomeInfo;
			trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["aktfno"]);

			if (trackNumber > Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["maxfno"])) {
				trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["minfno"]);
			}

			if (someVar == true) {
				s = "4512";
			} else {
				s = "9682";
			}

			s = s + Convert.ToString(trackNumber);

			writer.UppdateraFNO(trackNumber + 1);
			
			ContextUtil.SetComplete();
			return s + GetCheckSum(s);
		}
	}
}

Transactions

COM+ got a very good transaction handling if you understand it properly. There is really no need to invoke ContextUtil from within your methods (in most cases).

Transactions the right way

The transaction is completed when the entry COM+ component is disposed, that’s why disposing is important.

Take this example:

[Transaction(TransactionOption.Required)]
public class ComponentA : ServicedComponent
{
	public void SomeMethod()
	{
		using (var componentb = new ComponentB())
		{
			componentb.DoSomething();
		}
	}
}

[Transaction(TransactionOption.Required)]
public class ComponentC : ServicedComponent
{
	public void DoSomething()
	{
		using (var componentc = new ComponentC())
		{
			componentc.DoSomethingElse();
		}
	}
}

[Transaction(TransactionOption.Required)]
public class ComponentC : ServicedComponent
{
	public void DoSomethingElse()
	{
		// I really do something transactional
		// that might throw exceptions
		
	}
}

// your app
public static class Program
{
	public static void Main(string[] argv)
	{
		try
		{
			var component = new ComponentA();
			component.SomeMethod();
			//component.Dispose(); <--- See? Commented out
		}
		catch (Exception err)
		{
			Console.WriteLine(err);
		}
	}

}

Since we do not dispose component the transaction will probably timeout eventually, which means that it will be aborted.

But if we instead change to..

public static class Program
{
	public static void Main(string[] argv)
	{
		try
		{
			using (var component = new ComponentA())
			{
				component.SomeMethod();
			}
		}
		catch (Exception err)
		{
			// top layer, catching all exceptions is a-ok.
			Console.Writeline(err.ToString());
		}
	}

}

… we’ll get a transaction which is automatically committed or rolled back depending on if an exception was thrown or not.

Resulting code

After all refactorings we’ve got the following code:

public string SomeFunction(bool someVar)
{
	using (reader = new SomeReaderComponent())
	{
		using (writer = new SomeWriterComponent())
		{
			var ds = reader.SomeInfo;
			var trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["aktfno"]);

			if (trackNumber > Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["maxfno"])) {
				trackNumber = Convert.ToInt64(ds.Tables["trackNumber"].Rows[0]["minfno"]);
			}

			var result = someVar ? "4512" : "9682";
			result += Convert.ToString(trackNumber);

			writer.UppdateraFNO(trackNumber + 1);
			
			return result + GetCheckSum(result);
		}
	}
}
  • By removing all the exception handling we’ve got code which is easier to read since it focuses on business rules.
  • By declaring variables where they are used we’ve also made the code easier to read (since it reduces the number of lines
  • Wrapping all COM+ components with using makes sure that they are disposed properly
  • Not catching exceptions allows us to use the default COM+ transaction handling
This entry was posted in Architecture and tagged . Bookmark the permalink.