Amazon.com Widgets

ArgumentNullException and ArgumentException

In my comments Jeff asks:

I have a question, how was the decision for these 2 classes ctor parameters made...

Constructor for ArgumentNullException

public ArgumentNullException( string paramName, string message)

 

Constructor for ArgumentException

public ArgumentException( string message, string paramName)

 

Well, in short Jeff, this was a design mistake.  In fact it is up on the internal “API Design Hall of Shame”.  Here is the text from that site:

 

ArgumentNullException does not follow the constructor pattern given in the Design Guidelines spec.
Result: Habit wins out and people commonly type:
throw new ArgumentNullException (“must pass an employee name”);
rather than:
throw new ArgumentNullException (“Name”, “must pass an employee name”);
so we end up with error message such as:
Unhandled Exception: System.ArgumentNullException: Value cannot be null. Parameter name: must pass employee name
Lesson: Just follow the pattern!

 

Got any other examples that should be in my API Design Hall of Shame?

Published 15 February 04 08:05 by BradA

Comments

# David Bossert said on February 15, 2004 10:20 PM:
"Got any other examples that should be in my API Design Hall of Shame?"

Just off the top of my head:

AppDomain vs. ApplicationDomain
MarshalByRefObject vs. MarshalByReferenceObject
STAThreadAttribute vs. StaThreadAttribute
ObjRef vs. ObjectReference

...and so on. If I had my way, all of these annoying little naming mishaps would be fixed without notice and anyone who complained about compatibility would risk being beaten with a coat hanger... but that's just me.
# Sami Vaaraniemi said on February 16, 2004 12:01 AM:
My candidate to the Hall of Shame is IDbDataAdapter.Fill.

There is no override that would take a DataSet and a table name making it difficult to use it with typed datasets. In order to fill typed datasets, my database provider agnostic data layer has to use reflection to invoke the provider specific Fill with the table name parameter.
# Mike said on February 16, 2004 1:39 AM:
It is declared as

public static string SmtpServer {get; set;}

The MSDN documentation states:
Thread Safety
Any public static (Shared in Visual Basic) members of this type are safe for multithreaded operations. Any instance members are not guaranteed to be thread safe.

But, to send a message you need to set SmtpServer first (static property), and then send a message (static method). How should one do this "safe for multithreaded operations"?

Of course, if I write standalone application, this is not a problem - I can lock something. But if there is other code running in same process which can wish to send e-mail too, I am out of luck.
# RichB said on February 16, 2004 11:54 AM:
Any method in System.Security.Cryptography. Having written .Net code since 2000, it was a shock recently to use the Crypto APIs and find how bad they are.
# RichB said on February 16, 2004 11:55 AM:
CLSCompliantAttribute

- Doesn't follow the naming conventions. Should be ClsCompliantAttribute
# Ben said on February 16, 2004 12:57 PM:
I was wondering if there was any way to see this list - I think it'd be a great read. Wouldn't it also be neat to set a forum-like system up for the public to post their little oddities that they have found (like those posted here)?
# Kevin Westhead said on February 16, 2004 4:02 PM:
System.Windows.Forms.FileDialog has always annoyed me. This class is defined as:

public abstract class FileDialog : CommonDialog

However the documentation states:

"FileDialog is an abstract class, and cannot be created directly. Additionally, you cannot inherit from this class."

This seemingly contradictory statement is in fact true because FileDialog contains the following method:

internal abstract bool RunFileDialog(OPENFILENAME_I ofn);

The design here seems wrong and it is annoying when you have expectations (based on experience) about what you can do with a class defined as "public abstract", only to find out that the class behaves differently.

It is documented at least, which I suppose is something since you end up looking through the docs eventually. <g>
# _ said on February 16, 2004 6:00 PM:
RichB,

>> CLSCompliantAttribute

>> - Doesn't follow the naming conventions. Should be ClsCompliantAttribute

It does follow the conventions, according to http://blogs.msdn.com/brada/archive/2004/02/07/69436.aspx

Seeya
# Brad Abrams said on February 16, 2004 8:58 PM:
Nope, Kevin is right, it should be ClsCompliant... Three letters or more == Pascal cased.
# Atif Aziz said on February 17, 2004 3:49 AM:
It's interesting to note that a lot of complaints about casing issues can be fixed with a small feature that exists in the C# compiler today: Aliasing. That is, you can rename a type explicitly as you import it:

#region System aliases

using ClsCompliantAttribute = System.CLSCompliantAttribute;
using ApplicationDomain = System.AppDomain;
using MarshalByReferenceObject = System.MarshalByRefObject;
using StaThreadAttribute = System.STAThreadAttribute;
using ObjectReference = System.Runtime.Remoting.ObjRef;

#endregion

This is also do-able in Visual Basic.NET, which goes even further than C# if you dig into the books. What I never understood, on the other hand, is why the VB team left out aliasing of interfaces! Anyhow, just wanted to let some VB guy reading this comment to know that this solution is applicable there too.

Of course, it should not be our job to fix-up such mistakes and the improted list could look rather ugly at the beginning of each file (which is why it's good to hide it into a collapsed region), but it's nonethelss an option to consider if it really kills you to write AppDomain instead of ApplicationDomain.

By the way, aliasing is nice even if you don't want to rename a type. It is very useful for reducing the type noice in your scope. For example, why import the whole System.IO namespace if all you need is some method of the Path class? Here you would use aliasing as follows:

using Path = System.IO.Path;

Personally, I can live with such mistakes in the BCL with the hope that it will be fixed through some ingenious scheme of the CLR, compilers, the IDE or tools like FxCop. Hey, I've lived with some atrocities in Win32 for a big part of my career that the .NET Framework is pleasant experience any day. The quality bar though has to be raised for the next time as it was raised by the BCL with the first release. Looking at this blog tells me so far that someone is watching this space and a big effort is being made to pay attention to such details and fix them for the future. That's a good thing and keep up the great work, Brad. What's scary is the sheer size of the design guidelines, which seems to be growing by the day. This is a clear indication that we need to make things simpler, perhaps even question some of the basic notions as we understand them today, in order to make the next big leap.
# Dwayne said on February 17, 2004 12:19 PM:
Here's one for the hall of shame.

There is no base data access exception for the data access libraries. For example, the System.Data.SqlClient.SqlException derives from System.SystemException. So does the System.Data.OracleClient.OracleException class. This makes it more difficult to create a common data access interface that uses these classes internally but throws an implementation agnostic exception. It would have been nice if they had a System.Data.DataException that these exceptions derived from, so that my interfaces could be consistent.
# Dwayne said on February 17, 2004 12:20 PM:
The System.Collections API.

I'm surprised that the System.Collections.ICollection does not even have a Contains(object) method.
# Dwayne said on February 17, 2004 12:24 PM:
Actually, the most annoying thing about the System.Collections API is that CollectionBase implements IList. Most of the methods/properties in IList would be better defined on ICollection IMHO. At very least the Add, Clear, Contains, Insert, and Remove methods, as well as the IsReadOnly property.
# Tom McKearney said on February 17, 2004 1:00 PM:
Well, it exists. THey're just not using it the way you'd like.

http://msdn.microsoft.com/library/en-us/cpref/html/frlrfsystemdatadataexceptionclasstopic.asp
# Dwayne said on February 17, 2004 3:49 PM:
Good point Tom, that'll teach me to write my response without first looking at the APIs. The point I was making still stands though. There is no common base exception type for the database-implementation specific exceptions that can identify the exception as being one generated by the database.
# Atif Aziz said on February 18, 2004 2:27 AM:
Actually, talking about Exceptions, I think that the whole exception hierarchy is unreliable and broken in the .NET Framework. By unreliable, I mean that I cannot use it to make the right choices about the base exceptions and write code that can gracefully and predictably recover from a given situation while keeping forward and backward compatibility in mind. I think there is no real good story here. When is something just an Exception, when is it a SystemException and when is it an ApplicationException? I know that right thing for my own exception hierarchy is to base it off ApplicationException, but when I look at the .NET Framework, I get the impression that the various teams could not always agree on the best place to branch off their exception hierarchy. For example, some exceptions from the framework are based off ApplicationException. What makes things complicated are things like ArgumentException. This one inherits from SystemException, which means that if I inherit from it, then I am essentially making my exception a system one. The documentation states that, "[SystemException] is provided as a means to differentiate between exceptions defined by the system versus exceptions defined by applications." So the question is when are you a system and when are you an application? ArgumentException is probably not the best example because, frankly, no one would catch it at runtime since it probably means that you haven't tested things correctly and are just being plain paranoid. So a better case in point might be System.IO.IOException. If I am writing an I/O library and I want to create an exception hierarchy, should I base it off System.IO.IOException (which by the way inherits from SystemException) or create my own IOException that inherits from ApplicationException? Who is the system here? Is the SystemException space reserved for Microsoft? If that's the case, then the compilers should not allow anyone to inherit from SystemException or Exception.

Perhaps a better way to solve this problem would have been through attributes or marker interfaces. I don't believe that OO hierarchies are the best candidates for dealing with exceptions and the versioning issues that arise from it. The lack of consistency and a good story is what makes even some otherwise very clever piece of code to do something scary like this:

try
{
...
}
catch (Exception e)
{
...
}

Frankly, if you're not re-throwing the exception in the catch block then you've just broken everything because you'd end up even swalloing ridiculous exceptions like StackOverflowException, or God forbid, ExecutionEngineException. I can't imagine what this do for Yukon where various pieces of managed code need to run in a server reliably and 24 by 7. If they're swallowing system exceptions, then they're essentially hiding pretty grave situations from the host that normally lead to, say, the termination of their AppDomain.

Luckily, FXCop has a design rule that checks for the following violation: "You should not catch Exception or SystemException." However, I've found that in practice, you are at the pure mercy of how someone designed their exception hierarchy. If they based it off System.Exception, then you're left with no real choices. Also, use of FxCop is best practice and not mandatory.

Given this situation, I would be inclined to put the whole exception hierarchy into the API Design Hall of Shame.
# Kirk Allen Evans' XML Blog said on February 18, 2004 10:30 AM:
# Ken Brubaker said on February 20, 2004 10:48 AM:
For my own reference, I thought I'd compile a quick list of design guidelines added by Brad Abrams, et al.
# Diego Mijelshon said on February 20, 2004 9:01 AM:
Some of the biggest mistakes in API design are in the Collections Framework:

Why in the world System.Array implements IList.Contains as private?
Why do Arrays have a Length property instead of Count? (or vice-versa)

I could go on and on...
# Brian Grunkemeyer said on February 20, 2004 12:11 PM:
On these two questions:

*) Why in the world System.Array implements IList.Contains as private?

The thought was it was just syntactic sugar around IndexOf, or for sorted arrays, BinarySearch. Users figure this out themselves, and if they know their array is sorted, they can do this more efficiently than we could by simply calling the BinarySearch method. And if we didn't need to expose it, why add the surface area?

I was toying with suggesting that we make Contains public on Array, but I'm not sure that's the best decision for the product. In our Whidbey release, to support generic programming in a more natural way than C++'s Standard Template Library, we made T[] implement IList<T>. Of course there's a Contains method on IList<T>, which ends up calling Array's new IndexOf<T>. This can theoretically give better performance (with the right IL code generators and the appropriate work in our current JIT and/or a future JIT - measuring on any publically released V2 build right now is probably not a useful exercise). However, since we only made some arrays, not all arrays, implement IList<T>, we didn't define a Contains<T> on the Array class directly (there is one subtle internal hack where we violate the type system to make the type system useful for generic programming). So we haven't accidentally solved your feature request yet.

But there's no technical reason we couldn't add an Array.Contains<T> that is a simple wrapper around Array.IndexOf<T>. You would still be better off calling BinarySearch<T> if your array is sorted though. I'll run this by a few people - maybe this would be interesting & useful.


*) Why do Arrays have a Length property instead of Count? (or vice-versa)

Length made more sense for arrays, where they are always a series of values in memory. You can pin them & do pointer arithmetic on arrays of value types, etc. They're similar to C arrays, and we had that mentality firmly ingrained in us.

Collections can include data structures like trees or an infinite series of prime numbers. Neither of these has a length, and only one has a finite count. The naming issue here was too much to swallow, so we left arrays with Length and collections with Count. Yes, it's a bit confusing, but if this is the worst of the problems our users are running into, we've done a good job.
# Ken Brubaker said on February 21, 2004 10:50 AM:
Documentation bug for HttpServerUtility.Transfer
# SampoSoft .NET Blog said on February 21, 2004 3:10 PM:
# Diego Mijelshon said on February 24, 2004 5:17 AM:
Brian,

You're being a little too defensive with your product. Believe me, I'm not trying to destroy it; I just want to help you make it even better :-)

I just think it wouldn't hurt to have IList.Contains and IList.Count marked public instead of private.
I don't care if they actually call !(Array.IndexOf(this, value) < this.GetLowerBound(0)) and this.Length. That's just an implementation detail that shouln't change the API.
# Atif Aziz said on February 25, 2004 4:20 AM:
HttpException and all dervied exceptions are not marked as serializable! I can't imagine any reason for them not to be serializable. It's not like I want to persist the exceptions to disk, but lack of serializability in these classes means that they can't get tossed across remoting boundaries such as AppDomains and contexts. What's more, why on Earth do they inherit from System.Runtime.InteropServices.ExternalException, which is supposed to be, accoriding to the docs, the base exception type for all COM interop exceptions and structured exception handling (SEH) exceptions. Shame, shame, shame. :-) BTW, although Mono follows or has to follow the same exception hierarchy, it does at least mark HttpException as serializable.
# Brad Abrams said on February 28, 2004 11:26 AM:
Good point, Atif... I talked wth the dev team for HttpException. You will be happy to know they are making it serializable for Whidbey. The ExternalException base class is an oddness, but is harder to change.
# Brad Abrams said on February 28, 2004 4:20 PM:
Mike, on the issues with SmtpServer, you will be happy to know that in Whidbey we will be adding a much better mail support to the platform that does, I am told, fix this problem.
# Ilya Ryzhenkov said on February 28, 2004 9:29 PM:
My favorite is internal bool EndsWith(char value) in String class. Why it would be internal? Also, there is no bool StartsWith(char value).
# Mike Krings said on March 2, 2004 3:12 PM:
Publishing WMI Classes is nearly unrecoverable in case of a failure:

If the WMI class is not yet registered and the user is not in the Admin group, a simple System.Exception is thrown. No real chance to gracefully handle this, as the only hint on the source of the error is the error message.

If publishing fails because of an Security exception, the Exception raised is a COMException (shouldn't it be a ComException ?) with a WMI specific (not System.Management published) HRESULT of 0x80041003.

Looks like a real hasty transformation of the WMI API.
# RichB said on March 3, 2004 9:43 AM:
Random.Next(0,1);

does not return numbers in the set {0..1}. It returns numbers in the set {0..0}. Arghh!
# Kit George said on March 5, 2004 4:00 PM:
With regards to Ilya's question (why not add EndsWith/Startswith overloads that accept a char), there's simply not much of a gain here. We actually have a feature request for this, but it's lower down on our priority stack.

On Rich's Random.Next issue: yup, this is an unfortunate one. But basically, we can't change this behavior, for breaking change reasons. However, I just entered a feature request for us to consider either a new method, or a new overload (consider obsoleting this one).
# Adrian Florea said on April 4, 2004 3:25 PM:
Why this singleton System.Reflection.Missing has the constructor internal instead of private? Same question for the internal class System.Empty.
# Brian Grunkemeyer said on April 5, 2004 3:05 PM:
About the Missing & Empty constructors - there is no real difference. Whether the constructors are internal or private has no impact to the public API, especially since these classes are private. I suspect this was required in our original design, but never got fixed when we revamped these classes, about the same time we removed Variant from the public namespace about 2.5 years into writing V1. Either that, or it was an earlier compiler limitation. (We went through two separate compilers for V1 before we ported our source to C#.)

But this is a little odd, and I'll make these constructors are private.
# StrangeLog - Il blog di Andrea Saltarello said on September 13, 2004 11:13 AM:
# SampoSoft .NET said on September 14, 2004 9:00 PM:
# Brad Abrams ArgumentNullException and ArgumentException | Wood TV Stand said on June 1, 2009 1:52 PM:

PingBack from http://woodtvstand.info/story.php?id=1343

New Comments to this post are disabled

Search

Go

This Blog

Syndication

Page view tracker