Effective C#

Effective C# , now in its third edition is meant to be a professional developer's second book on C#. I assume you understand the basic syntax and idioms of the language. This book shows the techniques that will make the average C# developer a better C# developer.

You can purchase the third edition directly from the publisher here. If you prefer Amazon, you can buy here. The kindle edition is here.

Discussion Items from Effective C#

 

Question on Array Recommendations.

I finished your book a few days ago. It was a fun read, and I think you covered a lot of good areas for code improvement. I found one of your recommendations that contradicts with something I found in the Patterns and Practices Performance and Scalability book from Microsoft dealing with Multidimensional Arrays vs Jagged Arrays. You seem to be in favor of the Multidimensional Arrays over Jagged (Item 40, page 231-232). Here's what the P&P book has to say on that:

Chapter 5, page 241-242:

"Use Jagged Arrays Instead of Multidimensional Arrays

A jagged array is a single dimensional array of arrays. The elements of a jagged array can be of different dimensions and sizes. Use jagged arrays instead of multidimensional arrays to benefit from MSIL performance optimizations. MSIL has specific instructions that target single dimensional zero-based arrays (SZArrays) and access to this type of array is optimized. In contrast, multidimensional arrays are accessed using the same generic code for all types, which results in boxing and unboxing for arrays of primitive types."

They then get into some MSIL code comparing the two.

My Response:

This is one of those low-level performance issues where the performance metrics depend on the particular program. I'll get to the performance characteristics in a moment.

My overriding reason for recommending multi-dimensional arrays stems from ease of use, and maintenance. A multi-dimensional array is a single structure. Simulating multiple dimensions using jagged arrays is more complex: You create one array per row, plus the outer array. Quite simply, it's easier to get wrong, resulting in null reference exceptions.

This added complexity manifests itself in many ways. First, you write more initialization code (shown on p. 230-231 of Effective C#). Iterating all the members of a jagged array is more complex as well. A single for or foreach loop gets replaced with nested loops. I discuss this on p. 232-233.

While this may be simple code when you intend to model a multi-dimensional array using jagged arrays, there is no guarantee that all rows have the same number of elements. This also complicated column-wise traversals: You should check that each row has enough elements before simply assuming it works. If you want to support a single enumerator for a jagged array, you must write your own. It's already available in a multi-dimensional array.

These differences make me lean toward simplicity rather than speed. Quite frankly, a little slower but more maintainable code will scale better than overly optimized but harder to maintain code. That means I prefer the multi-dimensional array.

Finally, I'll address the performance issues. It depends on what you measure, and where your programs bottlenecks are:

A multi-dimensional array is one allocation; a jagged array causes N+1 allocations, where N is the number of rows. That has some cost.

Row-wise traversals will be faster for jagged arrays, as pointed out in the P&P guide.

Column-wise traversals will be faster for multidimensional arrays, as I point out in p. 231.

In all three cases, the differences will be small. You will only see the difference for an operation that is repeated very often. Before you make this kind of a change, you should profile code and determine which version is fastest. And, you should know that the speed difference is significant enough to justify the change.

.NET Framework changes affecting Items 9, 10

The advice is still proper, but the justification is somewhat different.

The 2.0 version of the .NET framework has changed the way they implement ValueType.Equals() and ValueType.GetHashCode(). This affects some of the justification for my advice in items 9 and 10.

I explained in Item 9 that these methods use Reflection to determine if the values stored are equal. Further, that as a performance improvement, only the first field in the value type was compared, instead of each field. A similar algorithm was used in ValueType.GetHashCode(): This method returned the hash code of the first field in the value type.

This behavior changes in 2.0. Beta 1 and Beta 2 now use all the fields in a value type to determine equality, and to calculate the hash code.

These changes do not fundamentally affect the advice I set forth in Items 9 and 10. (In fact, if you are already following my advice, the changes to the implementation of these methods will be transparent to you.) You should still override both methods, Equals and GetHashCode) for ValueTypes. While the implementations in the 2.0 framework are more correct than their predecessors, they will have a negative impact on performance because of their use of reflection. Furthermore, by creating your own implementation, you shield yourself against this type of version-to-version implementation changes.

Exceptional conditions about exceptions

A number of subtle points about exception processing.

This is one of those lengthy discussions that get filed under “There are always exceptions”. And, it happens to be related to the guidance I wrote on exceptions in Effective C#.

First, the simple question:

“Recently, I and fellow developers have a very heated and passionate debate on whether or not it is appropriate to derive one's application-specific exception from System.ApplicationException or System.Exception. (I was the lone voice, which now advocates the latter despite my past recommendation).
So far Microsoft and all its documentation recommend what you stated in the book and I have been following this recommendation. But of late, particularly in Whidbey Beta 1, the recommendation has changed. Even FxCop 1.312 for Whidbey has changed.
It now recommends the application-specific exception *should* derive from System.Exception instead.
I was initially furious with FxCop but after doing research, particularly SLAR (.Net Framework Standard Library Annotated Reference) and this URL go further to suggest the BCL team has admission of mistake: http://blogs.msdn.com/brada/archive/2004/03/25/96251.aspx

Well, yeah, we all goofed. It turns out that creating an application specific base class for exceptions doesn’t buy anything. In fact, it only makes writing good catch clauses harder. As I pointed out in Item 44, the main reason to create new exception clauses is to facilitate catching different errors and responding to them with different actions. Knowing that an exception was generated in application code vs. library code doesn’t really play into that decision. In fact, all it does is introduce an extra level of complexity in the class hierarchy. The world is a simpler place without using ApplicationException.

And now, the subtle correct behavior in Item 44:

“Also because Exception is derived from ISerializable, I don't seem to recall seeing the mentioning of ISerializable.GetObjectData(), which is mentioned in Item 25).”

Correct, I did not add a GetObjectData() method in any of the examples in Item 44. The reason is simple: None of these classes add any data fields not already found in the base class, System.Exception. For that reason the correct implementation of any derived GetObjectData would do nothing except call the base class, providing no extra functionality. I should have explained my reason for not including it, but the samples are correct without it.

And finally, the tough one:

“In relation to the use of Inner Exception, I have two minds about it. One is if you provide a form of abstraction where the client of your class/assembly needs not be aware of the implementation details. If the class designer wisely implements an Application-specific exception in the public interface (which exceptions are part of it) to support exception translation.

If the class designer unwittingly uses inner exception to embed the exception thrown by the implementation assembly, then this forces the client also to reference the implementation assembly in order to deserialize the application-specific exception. This to me is a break in abstraction. The problem is particularly acute in cross app domain calls, such as remoting. For instance class that support remoting service from a Data Access Layer. The client should not need to include say SqlClient in order to deserialize an exception thrown from the DAL.

The other area where the embedded inner exception can hurt is in those assembly that are loaded dynamically via Assembly.Load() using configuration details, generally in their own app domain. When the exception is deserialized it would also need to assembly supporting the inner exception to be available on the receiving side. Failure to have them can result in a deserialization exception.

In this usage, I tend to recommend to my fellow developers to avoid embedding exception. But rather constructed an exception message that include enough details of the implementation detail for technical staff to perform diagnostic.”

This is the difficult question, because it touches on many edge cases. First, let’s cover the normal case: You should fill in the InnerException property, because it’s how you preserve as much information as possible about the original error. Failure to do so means that the technical staff loses that information.

Now let’s look at the two cases my reader mentioned. First, let’s cover remoting. Ideally, you’d like to preserve the InnerException property. But, it’s just not safe. As the reader points out, if the implementation of the runtime class used in the InnerException isn’t available on the client, the client gets a SerializationException. That effectively loses all information in your original exception, so it doesn’t help anyone.

You might be tempted to look at the runtime type of the exception and include it as the InnerException property if it is delivered with the framework, and therefore available on the client machine. That’s a mistake, because you would need to walk all the InnerException property of each inner exception to ensure that you never find a type that might not be delivered to the client.

The Assembly.Load() gives similar results but for different reasons. Once again, you don’t know what domain into which you code will be loaded. If an exception type (or any other type, for that matter), gets passed across the domain boundary, your client process must catch serialization exceptions.

The root cause in both cases is that an object is being passed across process boundaries. As an aside, you can create the same problems by returning a derived class where a base class was specified. If the derived class implementation is not available to the client process, it cannot be serialized, and the client must catch serialization exceptions.

So, what should you do? Well, this is one of the reasons I prefer an SOA model, or web services implementation, to communicate between processes. The Xml Serializer is more resilient in these kinds of situations. In fact, you can’t really throw an exception from a web service to the client application. Instead, you return an error document.

If you have chosen remoting and you’re planning to stick with it, you just have to remember that the app domain boundary is always a special case, and all the concrete types that are passed through that boundary must be available on both sides of the boundary. As the reader mentions, you need to use some other technique to preserve exception information. My own preference would be to log all information about the exception on the server, using something like the Exception Processing block in the Enterprise Library. After that’s done, you can create a new exception that references the logged information and sends that to the client for client side processing and reporting.

Inter-process (and app domain) boundaries are edge cases, and you need to take special care with information that crosses those boundaries, including exceptions.

Errata

Sadly, every author makes mistakes.  Here are links to the errata pages:

` Edition

Item 3, p. 13: Update to Sample

The second sample does not show the null check described in the text.  It should read as follows:

 try
{
MyType t;
t = (MyType)o;
// Fails. o is not MyType
if (t !=
null)
{
// work with T, it's a MyType.
}
}
catch (InvalidCastException)
{
// report the conversion failure.
}

Item 3, p. 17-18

The sample that starts at the bottom of p. 17 and continues to the top of p. 18 should read as follows:

 object o = Factory.GetObject();

MyType t =
null;
if (o
as MyType !=
null)
t = o
as
MyType;

The sample in the first printing is repeated from the previous sample.                   

Item5, p.35

The single line code sample that reads:

Console.WriteLine(string.Format(new CustomFormatter(), "", c1));

Should read:

Console.WriteLine(string.Format(new CustomFormatter(), "{0}", c1));
Note the addition of the {0} to the call.        
Chapter 4 Introduction, p. 179

“crated a roundtable” should be “created a roundtable”

Item 9, p.59-60.

This snippet span the bottom of p. 59 and the top of p. 60:

 Circle c = new Circle(new PointF(3.0f, 0), 5.0f);
Flatten(c);
// Work with the circle.
// ...

// Convert to an ellipse.
Ellipse e =
new
Ellipse(c);
Flatten(e);
It should read:
 Circle c = new Circle(new PointF(3.0f, 0), 5.0f);

// Work with the circle.
// ...

// Convert to an ellipse.
Ellipse e =
new
Ellipse(c);
Flatten(e);

Item 16, p. 95

The last sentence in the 2nd to last paragraph references the wrong item number.  It reads “Item 18 explains how to do just that [Implement the Dispose pattern].”  It should read “Item 17 …”.

Variable typo on p. 106 (Item 18)

The sample near the top of p.106 reads:

public MyData Foo2()
{
  return myData.CreateCopy();
}
// call it:
MyData v = Foo();

 It should read:

public MyData Foo2()
{
  return myData.CreateCopy();
}
// call it:
MyData v = Foo2();

Furthermore, in the next paragraph, I refer to the “CreateCopy” method as “Clone”. That was a mistake, and should be the “CreateCopy” method.

Variable typo on p. 119

This code snippet:

// remaining details elided
public string Line1
{
    get { return Line1; }
}
private readonly string line1;

Should be:

// remaining details elided
public string Line1
{
    get { return line1; }
}
private readonly string line1;

Sadly, the code as shown in the book, will have an infinite loop.

Item 23, p. 139-140

There are a couple code samples that do not correctly reflect the text.  First, the sample on the bottom of p. 139 must include the ‘new’ keyword on the Message() method:

 public class MyDerivedClass : MyClass
{
public
new
void Message()
{
Console.WriteLine(
"MyDerivedClass"
);
}
}

Secondly, the declaration of MyDerivedClass on p.140 should include the IMsg interface:

 public class MyDerivedClass : MyClass, IMsg
{
public
new
void Message()
{
Console.WriteLine(
"MyDerivedClass"
);
}
}

Item 27: Page 161

The sample at the top of the page should not have the private constructor added. That’s discussed in the next sample on p. 162.

Item 29, p. 176

In the second to last sentence, “an delegate” should be “a delegate”.

In the last paragraph, the sentence that reads “The GetAnItemLater() method probably is a bit more confusing” should read “The GiveAnItemLater() method probably is a bit more confusing.

Item 34, p. 199

The snippet near the bottom of the page that reads:

"var obj1 = new B();
obj1.Bar(new D2()); "

should read:

"var obj1 = new B();
obj1.Foo(new D2());"

 

Item 36, p. 216

In the last paragraph on page 216, it reads “PLINQ will use a fixed number of threads, whereas AsParallel() will ramp the number of threads up or down to increase throughput.”

This is somewhat misleading.  Both AsParallel and Parallel.ForEach will determine an optimal partitioning and an optimal number of threads based on knowledge of system resources (primarily how many cores are available). The difference is that the ParalelEnumberable class contains different methods that enable you to provide custom partitions.


First Edition

Poor word choice in Item 3

At the bottom of page 18, in Item 3, I wrote this sentence:"If a particular object is not the requested type or is derived from the requested type, they fail."A better choice would have been:"If a particular object is not the requested type, nor a type derived from the requested type, they fail."

P 19, (item 3)

The code at the bottom of p. 19 reads:

//Version two

try
{
   MyType t1;
  
t = (MyType)o;

It should be:

try
{
   MyType t1;
  
t1 = (MyType)o;

 Errata: p. 24

The sentence "The as operator returns true ..." Should read "The is operator returns true ..."

The sentence that says, "If you made a create a Newtype" should be "If you made a new class derived from Newtype"

Errata: Missing words on p. 144

p. 144, Item 24, contains the following sentence:  "The DefaultSort attribute e, the Name property."

What should be there is this:

"The DefaultSort attribute defines the default sorting property for the Customer class

in this case, the Name property."

error in Item 5, p. 37

On page 37, in item 5, the line of code that reads:

    Console.WriteLine( string.Format( new CustomerFormatter(), "", c1 );

should read:

    Console.WriteLine( string.Format( new CustomerFormatter(), "{0}", c1 );

 Errata: Item 10

Item 10 incorrectly implies that System.ValueType contains a static operator ==.

It doesn't. 

However, many developers do create what seems like a simple solution:

struct MyType
{
  // data members elided.
  static bool operator == ( MyType l, MyType r )
  {
    return l.Equals ( r );
  }
  // Note no override of Equals.  
}

That will cause the behavior I mentioned in the item. The reason is that ValueType.Equals() does use reflection to find the values of each field in the objects being compared.

p. 122 (Item 19)

The second sentence in the third paragraph reads:    "It [IListSource] also has a containsListCollection property so that  users can modify the overall structure of the collection."

It should read:  "It [IListSource] also has a containtsListCollection property so that users can navigate the overall structure of the collection."

(Bold indicates the change, not emphasis). 

Clarification: Item 22

Elided code was not clearly marked
Both listener classes on p. 134 should include a private declaration of the Logger Singleton class:


        private static Logger logger;


This declaration was removed from the code listing, but not discussed in the text.