API design hall of shame...

Here is a new one for the API design Hall of Shame…. It has been in the .NET Framework from V1.0 days and I can’t believe I didn’t know about it until today… Maybe I did know about it before but I blocked it out of my memory ;-)

This API takes “true” and “false” rather than a Boolean value true and false…

https://msdn2.microsoft.com/en-us/library/system.appdomainsetup.shadowcopyfiles.aspx

public string ShadowCopyFiles { get; set; }

A String containing the string value "true" to indicate that shadow copying is turned on; or "false" to indicate that shadow copying is turned off.

Obviously, I would have MUCH preferred

public bool ShadowCopyFiles { get; set; }

 

So I checked with the team that owns the API to get the “background” story… While no one that still works in the area would claim “credit” for this API, I did get the story. Apparently the rationale for using string was that the underlying infrastructure uses strings rather than bools. This is a classic argument and one you should be very wary of whenever you hear it. For the purpose of great API design you should separate your self from the implementation details for a moment and just ask “what would the user like\expect”.. and only deviate from that where there is a super compelling reason. There clearly is no in this case.

Other suggestions for the API design all of shame? Hopefully no new ones in .NET Framework 2.0.

Comments

• Anonymous
  November 17, 2005
  Classic!
• Anonymous
  November 17, 2005
  Can't you just track down the guilty by looking in your source control system? Surely you're running an ssc with history...
• Anonymous
  November 17, 2005
  One thing that I find weird is how the Bulleted List works. It does NOt take the ID from the datasource as it should making the DataValueField absolutely useless. I posted it as a bug and they came back with the usually "that's by design" reply. Uh huh... that sooo would not pass usabiltiy tests. I need to be able to click on somethign and do something with it via id not a numeric sequence which means nothing to me. I either end up writing some custom solution or making a lookup take to translate the sequntial numbers it gives me to the REAL numbers which DataValueField needs to bind to.
  
  Anyhow... that's been driving me insane for a long time,
• Anonymous
  November 17, 2005
  We've got a few embarrassing ones over in Debugger land.
  Your example is a case of it being "ugly". But there's also a whole level of stupidity where the "design" makes something impossible.
  
  For example, there's a design flaw in the debugging API such that we can't report managed Exceptions that occur at startup. (Eg, if a thread throws a TypeLoad exception before it actually enters managed code. Details here: http://blogs.msdn.com/jmstall/archive/2005/08/01/Exception_Design_flaw.aspx )
  
  But at least this isn't new in V2 :)
  
• Anonymous
  November 17, 2005
  Sorry Brad, yes here is a new one in v 2.0, at least in my view.
  
  It is about XPathExpression.Compile(string,IXmlNamespaceResolver), and I already submitted that at least the documentation should be clarified (FDBK40597).
  
  Whenever someone wants to use custom functions or variables in an XPath (but not XSLT) based solution, one has to pass a 'context' that derives from XsltContext to the expressions to be evaluated (the fact it is named XsltContext and not 'XPathContext' is in itself already wrong IMHO, to me that indicates that whoever designed the XPath parts was ignorant about the fact that XPath with variables and custom functions has plenty of uses outside XSLT).
  
  The overload of XPathExpression.Compile mentioned above seems to be the perfect way to pass that context, after all XsltContext implements IXmlNamespaceResolver. An innocent programmer may think that the following code would do the trick:
  
  MyXsltContext ctx = ...;
  string xpathtext = ...;
  XPathExpression expr = XPathExpression.Compile(xpathtext, MyXsltContext);
  
  Sadly, a look with Reflector shows that XPathExpression.Compile creates a new context, copying all namespaces from the one that was passed, and setting that new context (which of course has no knowledge of variables and custom functions) as the expression's context...
  
  The way to get it working as intended is
  
  MyXsltContext ctx = ...;
  string xpathtext = ...;
  XPathExpression expr = XPathExpression.Compile(xpathtext, null);
  expr.SetContext(MyXsltContext);
  
  
• Anonymous
  November 18, 2005
  Yeah, I thought that one was kinda strange. Speaking of that, the docs say that AppDomain.SetShadowCopyPath is being deprecated and recommend using AppDomainSetup.ShadowCopyDirectories instead. The problem is that the SetupInformation on AppDomain gets a copy of the SetupInformation, so there is no way to modify the shadow copy path of an existing app domain. I have a scenario where this is useful, so right now, I'm forced to use the deprecated member. Do you know what I'm missing, or is this an API flaw?
• Anonymous
  November 18, 2005
  Not sure it counts, but why doesn't treenodes and its ilk use the CheckState enumeration instead of a boolean property? The obvious answer is that the Windows Forms version of the control doesn't support the tri-state checkbox...which of course begs the follow on question of why not?
• Anonymous
  November 18, 2005
  My nomination would be list item attributes only being rendered within htmlxxx controls, and not for radiobuttonlist, dropdownlist, etc. This was very counter-intuitive for me the first time I ran into it.
• Anonymous
  November 18, 2005
  The comment has been removed
• Anonymous
  November 18, 2005
  ShadowCopyFiles is worse than even you describe here. On v1.x, shadow copying will be enabled if ShadowCopyFiles is set to any non-null string value; even the empty string.
  
  So doing something like this:
  
  AppDomainSetup adSetup = new AppDomainSetup();
  adSetup.ShadowCopyFiles = "false";
  adSetup.ShadowCopyDirectories = "c:\blah";
  
  actually enables shadow copying, which is (to be polite) counterintuitive.
  
  This has been fixed in v2.0. Only "true" will enable shadow copying.
• Anonymous
  November 18, 2005
  As a follow-up to my earlier comment, I'd like to point out that this is yet another potential security vulnerability caused by a class not being sealed.
  
  It's easy to imagine that someone writing a trusted library would accept a List<> object as the return value from an untrusted method and then assume that the values returned from the IList<>.Item property behave correctly (e.g. always return the same value). Note that a stack walk wouldn't protect against this.
  
  The other vulnerability I'm referring to is that if a bad guy can declare a subclass D of a trusted class C, then they can create MemberwiseClone()s of D objects. I can easily imagine that the author of C assumes that they control instance creation, and base their security analysis on that assumption. E.g. when the object holds a counter of a maximum number of times some action may be performed. Cloning the object creates a new counter, allowing the action to be performed more often than anticipated.
  
  So, there's another Hall of Shame hopeful for you: MemberwiseClone().
  
  More generally, I believe classes should have been sealed by default in C#. Instead of a sealed keyword, there should have been an extensible keyword.
• Anonymous
  November 18, 2005
  Larry Osterman had a post some time back about GetBadWritePtr (and friends), and how they're essentially useless and potentially damaging. Those would be good candiates for the hall of shame.
• Anonymous
  November 18, 2005
  I vote for the entire System.Diagnostics.SymbolsStore namespace. I have tried five times and can't figure out how to use it for anything (if it is even possible). What makes it worse is that it is so tempting, because it sounds like it would be very useful for reading .pdb information...
• Anonymous
  November 18, 2005
  I have a canditate for the hall of shame - DBNull class. Not serializable, pain-in-the-neck to deal with class that shouldn't have existed in the first place (plain null reference would do).
• Anonymous
  November 18, 2005
  Here's one I just discovered:
  vsCommandStatusNinched
  
  Ninched?
  
  
• Anonymous
  November 18, 2005
  Here's one I was just bitten by. This constructor for the Binding class
  
  Binding (String, Object, String, Boolean)
  
  The docs say that the last parameter "optionally enables formatting to be applied". What is not obvious is that the BindingComplete event does not fire either unless it is set to true!
• Anonymous
  November 19, 2005
  Douglas - see http://blogs.msdn.com/jmstall/archive/2005/08/25/pdb2xml.aspx for a sample that uses the symbol store interfaces to dump a PDB file into XML.
  
• Anonymous
  November 19, 2005
  How about Object.GetHashCode()? It seems to be there solely to make hashtables work, which is strange. The implications of overiding .Equals() is greatly complexified (is that a word) by the unnecessary coupling of that API with .GetHashCode(). In fact, there really is not a clear documented way to override .Equals in a mutable object that will cause the object to work correctly in a hashtable()!
  
  Why wasn't .GetHashCode() left off of object and a different pattern chosen for hashtables, such as a required interface for objects added to hashtables or a way to refer to a hashcodebuilder? What percentage of object types in a typical application are actually used in a hashtable anyway? I would have preferred to make .Equals() much easier to override and just create an easy way to make objects hashable for a hashtable.
  
  
• Anonymous
  November 20, 2005
  Another one for the .NET 2.0 hall of shame:
  
  SqlDependecy.OnChange event
  
  OnChange event? Some one ignored the
  
  public event Event
  
  protected void OnEvent
  
  pattern.
  
  When it was discovered, it was too late to correct. Yeh!
• Anonymous
  November 20, 2005
  I got to say, speaking of the "that's the way it was" argument, GDI+ must be the ugliest thing I've had to deal with in .NET. The "Rectangle" vs. "RectangleF" everywhere and the lack of structs being immutable is a real killer.
• Anonymous
  November 20, 2005
  Image.Palette is a horrible API. It does not work the way anyone I've ever talked to or worked with would have expected. As a result, almost everyone ends up giving up on trying to do indexed bitmaps. I've written more about it here.
  
  http://spaces.msn.com/members/stefanrusek/Blog/cns!1pmxsvG-1DbBBuajMN2jpMqw!192.entry
• Anonymous
  November 21, 2005
  Actually, Bart, I'd say that the List interfaces are too monolithic, and need to be split apart.
  
  With the current set of interfaces, there's no way to express whether or not an enumerable is finite in length or infinite, and if it's finite, whether or not you can state the length ahead of time.
  
  Instead, you've got a bunch of APIs whose default behavior is to throw a not-implemented exception. Instead, I think, they should have been left off, and moved to interfaces that could optionally be implemented only on those classes for which they made sense. (For example, not on a serial port's stream.)
  
  While on the subject of streams (and message queues.. let's not forget those), couldn't we have added an IEnumerable interface? I'm learning to love the usefulness of IEnumerable<T>, and the following would be very nice:
  
  SerialPort port = ...;
  foreach (byte? b in port.Stream) // Stream -> IEnumerable<byte>
  {
  if (!port.IsOpen) break;
  Console.WriteLine("{0:x2} ", b.Value);
  }
  
  .. Though perhaps a stream would be better expressed as a Queue.
  
• Anonymous
  January 07, 2006
  Here's a good one: the last two values of this enumeration:
  http://msdn.microsoft.com/library/default.asp?url=/workshop/networking/pluggable/reference/enums/bindstring.asp
  
  (Nevermind the fact that most of the values are documented as "not currently supported".)
  
  Developers writing a pluggable protocol handler can call IInternetBindInfo::GetBindString to get various string values relating to the URL binding. This was introduced with IE4. Well, in IE5 there are some other types of data (non-string) you might want to get back. Rather than create a new interface with a new method, they overloaded this existing method to "string-ize" the values you might want.
  
  The documentation is so sparse it doesn't even say what format these last two strings will be in. I happened to run across them in WinDbg and found out what they were. BINDSTRING_FLAG_BIND_TO_OBJECT returns a string "TRUE" or "FALSE". And BINDSTRING_PTR_BIND_CONTEXT returns a string of the address of the IBindCtx in decimal form, e.g. "1101284".
  
  Definitely API hall of shame material.
• Anonymous
  June 08, 2009
  PingBack from http://jointpainreliefs.info/story.php?id=2317