Errata for the Framework Design Guidelines
We are finalizing plans to do a 2nd printing of the Framework Design Guidelines (thanks for those of you who bought a copy)… As you may be aware, with additional printings they don’t generally like authors to add new content, but it does afford an opportunity to fix small errors. Krys and I have found a few of on our own, but we’d love to hear your feedback as well. What errors have you found? While I can’t promise to send you $2.56 like Knuth does I will be grateful and will dually credit the first reports of such errors on my blog.
Thanks for your help!
Comments
- Anonymous
November 18, 2005
Hi Brad. One thing I noticed is that in the basic dispose pattern, the guidelines make no mention of calling base.Dispose(bool); when overriding a class that uses this pattern. Wouldn't that be necessary for the base class to clean up its resources?
I wrote up the issue here: http://haacked.com/archive/2005/11/18/11222.aspx
Also, can you call methods on your base class when within a finalizer? - Anonymous
November 18, 2005
pg 265 first paragraph third sentence.
Also, using optional features exposed <whoops!>though</whoops!> interfaces...
I found another typo, but forgot to write it down and now can't seem to find it. - Anonymous
November 21, 2005
Haacked, in response to the two issues you brought up on your first reply:
(1) Failure to mention "chain-to-base" for Dispose;
(2) Question about calling methods on base within Finalize.
Re (1):
If we neglect to say a derived class must chain to its base class Dispose method (if it has one), that’s a book-bug/omission. If you're writing a Dispose(bool) your preference is to call base.Dispose(bool), flowing the bool value to the base method; but if one doesn’t exist, a base.Dispose() suffices; otherwise, you simply call base.Dispose().
This is important because Dispose implies cleaning up resources; if you don’t chain to the base type, clearly you are going to leak those resources. And it might be worse than just nondeterministic cleanup (when the user expected deterministic). Presumably Dispose on the derived class does a GC.SuppressFinalize(this), meaning the base type will never get placed into the Finalize queue, and thus won't release its resources (well, until process exit). The user of this class would notice this as unbounded resource consumption. I suspect the bug would be incredibly hard to find, too.
Re (2):
Generally, you should not make virtual method calls from your Finalize method. This is to avoid accidental reliance on a resource which has already been disposed during the destruction process. The original document from which the book text was derived acknowledged that Dispose's use of this practice is risky and goes against the general advice. (Available here: http://www.bluebytesoftware.com/blog/PermaLink.aspx?guid=88e62cdf-5919-4ac7-bc33-20c06ae539ae)
But we made an exception for Dispose because of the carefully controlled pattern. (This was in the original document.) And virtual calls during destruction aren’t nearly as dangerous as virtual calls during construction. You’re typically concerned that a resource will be used before it’s been initialized (i.e. in the construction case), but presumably code called from your Finalize will be resilient to uninitialized state and isn’t going to make further virtual methods (which might introduce problems). Clearly if this isn't the case--and it could be difficult to verify that it is through test coverage--you will run into bugs, perhaps manifesting as crashing the Finalizer thread.
joe - Anonymous
December 13, 2005
Page 60 (comment in 2nd code block):
// Image prefix is not necessary
should be
// ImageMode prefix is not necessary - Anonymous
December 28, 2005
Not sure if this is an errata or just my lack of understanding, but I was surprised by the code example for 8.3, guideline 2 ("DO NOT use ArrayList or List<T> in public APIs").
Showing the return of Collection<> vs List<> makes sense conceptually, but I would have expected to see ICollection<> instead, since List<> doesn't derive from Collection<>, and you don't want to imply snapshoting the internal list just to avoid exposing the richer type, right?
-Nadim - Anonymous
January 31, 2006
The comment has been removed - Anonymous
January 31, 2006
The comment from KRZYSZTOF CWALINA, "... or even the wrong type names..." It would be terrible to use wrong type names in public API.
Shawn Cheng - Anonymous
February 02, 2006
On page 75, first sentence of third paragraph, "Finally, reference types are passed by reference, whereas value types are passed by by value."
I would think that both reference and value types are passed by value (the value of the reference and the copy of the value type). One can pass both types by reference by using the ref keyword.
Shawn Cheng - Anonymous
February 02, 2006
Sorry for not mentioning on which page KRZYSZTOF CWALINA gives the comment. It is page 54. - Anonymous
February 02, 2006
Section 4.4, "Abstract Class Design"~
I would think that if the guideline treats "internal" as same as "public" as far as different users working on the same assembly is concerned, then the first "DO NOT" and "DO" should be:
DO NOT define public or internal constructors in abstract classes.
DO define a protected or a protected-internal constructor in abstract classes.
If the guideline does not support "internal", then they should be:
DO NOT define public constructors in abstract classes.
DO define a protected constructor in abstract classes.
Please do correct me if I'm wrong here.
Shawn Cheng - Anonymous
February 02, 2006
It seems that the CONSIDER on page 55 is not in sync with the last paragraph on page 54. My opinion is that we should consider usage scenario first, then if it adds value, consider using the name of the base class as suffix.
Shawn Cheng - Anonymous
February 02, 2006
Typos: page 77, two "get { ... {" should be "get { ... } - Anonymous
February 02, 2006
Page 99, "if ( ... === ... )".
Shawn Cheng - Anonymous
February 04, 2006
I would think, to be more specific, the CONSIDER on page 87 should be:
CONSIDER defining an interface if you need to support its functionality on classes that already inherit from some other class.
Shawn Cheng - Anonymous
February 04, 2006
I guess the word "filed" from the last sentence on page 89 is a typo.
Shawn Cheng - Anonymous
February 04, 2006
In my opinion, the first DO on page 89 is kind of strict to follow. Should it be a CONSIDER instead of DO?
A framework should provide one or more implementations for the interfaces it defines, but it might not need to consume every its own interface in any way.
Adding in extra APIs to the framework just to consume its interfaces is not a good solution to validate the interface design. The more we think about the purpose of those extra APIs, the more issues might be raised. For example, what access scope should we give to those APIs? If they are public, they can be invoked by the users of the framework. But do those APIs have solid purpose, so that the users want to call them? If the APIs are private, then the framework is forced to call them in some way, even if those extra APIs only serve as test cases. This will affect the design of the framework.
In my opinion, the developers of the framework should provide test cases to consume the interfaces defined by the framework during development, and provide sample code (examples) to the users to show the scenarios to use the interfaces.
Shawn Cheng - Anonymous
February 06, 2006
Page 171, the comment from JEFFREY RICHTER, "Close" (typo) should be "Clone".
Shawn Cheng - Anonymous
February 06, 2006
On page 193, I would think there are typos in the comment from CHRISTOPHER BRUMME. The first sentence should read:
"If you use [finally] clauses for cleanup, you should know that any code that comes after the end of the [finally] might not be executed."
This happens when there is an exception. If you use catch clauses for cleanup instead, and if you can catch the exceptions concerned, your code after the catch will definitely run.
Shawn Cheng - Anonymous
February 06, 2006
Not so sure whether this is a b-u-g, but could anyone please help point out the difference between "Do prefer" and "CONSIDER".
Shawn Cheng - Anonymous
February 06, 2006
On page 197, I would agree with JEFFREY RICHTER at some degree, and would think that if the DO is changed to CONSIDER, many of us would feel better.
Shawn Cheng - Anonymous
February 06, 2006
CONSIDER on page 186 refers to the wrong section.
Shawn Cheng - Anonymous
February 07, 2006
Page 107, the comment from BRAD ABRAMS, I guess there is a typo in the last sentence, which should read:
As you see in section 5.7.1, [an enum] argument would make it easier to understand code calling this API.
Shawn Cheng - Anonymous
February 07, 2006
AVOID on page 156, the first sentence should read:
"Using out or ref parameters requires experience with pointers, understand how pass-by-value and pass-by-reference differ, and handling methods with multiple return values.
Shawn Cheng - Anonymous
February 07, 2006
I would suggest to give every single rule in the guideline a number for easy discussion.
Shawn Cheng - Anonymous
February 08, 2006
Given section 5.7.5, "Pointer Parameters", I would think that the DO NOT on page 149 should be "AVOID" instead. Or it should be broken into two sub rules, one is DO NOT, another AVOID.
Shawn Cheng - Anonymous
February 08, 2006
The comment has been removed - Anonymous
February 09, 2006
The comment has been removed - Anonymous
February 09, 2006
[page 152, ANTHONY MOORE]
I can't find CodeTypeReference.IsGlobal from MSDN help.
Shawn Cheng - Anonymous
February 09, 2006
[page 153, first DO]
As the example does not accept null as value for item, the method Add should check at the beginning whether item is null to throw ArgumentNullException.
Shawn Cheng