API Overloads: Don’t overload different behaviors!

Clarity of an API’s intent and behavior is critical when you’re developing an API, be it internal or external. Overloads seem like a sexy way to pile up a lot of options for your API’s consumers; however, you’re doing no one any favors by overloading behavioral changes based on differing parameter combinations.

Much of the .NET framework is awful about this; SharePoint’s APIs take that a horrific step further. It’s Emeril’s “BAM!” on crack, but not as cute and charming.

Today I spent far too long trying to decipher why I wasn’t able to upload a file to a SharePoint document library. Two different usages of a call to SPList.RootFolder.Files.Add(), two completely different results.

This call caused an Access Denied exception to get thrown:

DateTime created = DateTime.Now;

DateTime modified = DateTime.Now;

bool overwrite = targetList.EnableVersioning;

SPFile newFile = targetList.RootFolder.Files.Add(name, file, ht, author, author, 

                                                created, modified, overwrite);

This call successfully uploaded the file:

bool overwrite = targetList.EnableVersioning;

SPFile newFile = targetList.RootFolder.Files.Add(name, file, ht, overwrite);

So on top of some awful breaking of the Law of Demeter (code pulled straight from too many crappy MSDN examples by some other dev), these two calls look and behave completely differently.

Have a look at the documentation for SPFilesCollection.Add. It has 24 overloads. Twenty-frickin-four, ladies and gentlemen. The signatures vary from two parameters to eight – some with two DateTime and Boolean parameters. Worse yet, there’s no indication in any of the overloaded descriptions that they’ve differing requirements or prerequisites. (Thankfully the SharePoint crew named the parameters somewhat clearly so it’s somewhat helpful deciphering intent while in the IDE.)

After a couple hours of troubleshooting, I found the first invocation was successful when the invoking user had Full Control rights to the doclib but would fail when the user only had Contribute.

The documentation didn’t provide any help, so I spent a bit of time trying to understand why the two different results. I can only assume (and I’m using the word advisedly) the first method requires the additional privileges to set properties about the file as it’s being uploaded. This is my complete assumption because, as you may have noticed, the documentation wasn’t any help.

So stepping back from my regular rants about SharePoint’s awful developer experience, let’s have a look at the root problem: two different invocations of the same method with completely different behaviors behind them. #FAIL.

The right way to do this would be to NOT overload those particular calls and instead refactor them out to completely different calls. That first method call should instead look something like

SPFile newFile = targetList.RootFolder.Files.AddAndModifyFileProperties(name, file, ht,

    author, author, created, modified, overwrite);

(Not to mention the documentation should clearly state permission requirements for calls…)

Overloads are absolutely appropriate when you’re offering up the same behavior (see your favorite unit test framework’s overloads for AreEqual, etc.) – but overloads are flat evil when you’re offering them up and backing them with completely different behaviors.

Please don’t overload methods with behavioral changes. If you’re feeling that evil go kick a kitten or promote Richard Simmons for President. Just don’t do the overloads. Please.

UPDATED: Whoops. I was so fired up I confused the Liskov principle with the Law of Demeter. Thankfully Jim Weirich pointed out my error! Corrected.