API Design: how to lie less
So I wrote a bit last week about the importance of not lying through your APIs, but I didn't offer any examples on how to improve the examples I gave.
Let's take one at a time and consider how they might get improved.
The first case we had was the mis-named API.
public static void WriteToFile(Customer customer, string path)
{
// Actually, write the orders of the customer to the file.
// ...
}
You can fix this simply by having the method name describe what the API does.
public static void WriteCustomerOrdersToFile(Customer customer, string path)
{
// Write the orders of the customer to the file.
}
This may lead you to refactor the code a bit, once you understand better what it does.
public static void WriteOrdersToFile(
IEnumerable<Order> orders, string path)
{
// Write the orders of the customer to the file.
}
The next case was an API with fine print.
public static void WriteToFile(Customer customer, string path)
{
// Write the customer but only if we have permissions to the
// path; otherwise write to a temp file and set an environment
// variable to indicate we did this.
// ...
}
When addressing these, it is often a good choice to refactor things such that you end up with simple APIs that do straightforward work, and then other APIs that consume them with more nuances. Note that I've removed the communication through environment variables and instead I return an explicit out parameter. This changes behavior a bit, but you can have the caller of this API set it if you still need it.
public static void WriteToFileWithFallback(
Customer customer, string path,
out string tempFileName)
{
if (CanWriteToFile(path)) {
tempFileName = null;
} else {
tempFileName = CreateTempFileName();
path = tempFileName;
}
// This won't handle any special cases.
WriteToFile(customer, path);
}
Of course, if you can get rid of the fine print altogether that's usually the best choice.
Next, we have the case of the API that does a lot more than you asked for.
public static void WriteToFile(Customer customer, string path)
{
// Write the customer to the file, and if it succeeds, compress
// the file and delete any temporary files that may have been
// left over from interrupted file writes in the past.
// ...
}
The first step is usually to break this up into meaningful, lower-level bits of functionality you can then combine.
public static void WriteToFile(Customer customer, string path)
{
// Only writes to file.
}
public static void CompressFile(string path)
{
// Only compresses the file.
}
public static void RemoveUnusedTempFiles(string path)
{
// Deletes files that are associated with the given one and no longer
// needed.
}
Next, it's possible that you just had a naming problem. In this case, this operation looks a lot like what would happen when a user presses the Save button in an application that may have auto-save and other similar features. If this is the exact mapping we need and our policies are unlikely to change, it might be a simple matter of having a renamed entry point.
public static void InvokeSaveOperation(
Customer customer, string path)
{
WriteToFile(customer, path);
CompressFile(path);
RemoveUnusedTempFiles(path);
}
Another option is that the operation was doing many things because it really was a more complex beast. If so, sometimes you may choose to make the operation a class of its own, where you can configure it in different ways to control the more subtle aspects, and have a richer description of what should happen and possibly what the results were.
public class CustomerWritingOperation {
public bool EnableCompression { get; set; }
public bool CleanupTempFiles { get; set; }
public Customer customer { get; set; }
public string path { get; set; }
public void Run() {
WriteToFile(this.Customer, this.Path);
if (this.EnableCompression) {
CompressFile(this.Path);
}
if (this.CleanupTempFiles) {
RemoveUnusedTempFiles(this.Path);
}
}
// Other things could come here, like progress notification, etc.
}
It also has the nice benefit of allowing for cleaner ways to extend it with more functionality and with things like progress reports and callbacks.
Finally, we have the teasing API example. In this case, we have an API that promises more than it will deliver.
public static void WriteToFile(Customer customer, string path)
{
// Write the customer to the file but only handling the case where
// the customer is not a SpecialCustomer subtype; if so, throw
// an exception.
// ...
}
The most straightforward way to stop lying here is to implement the missing functionality. Maybe you didn't have time the first time around to handle special customers, but now you've found the time and you can round out your API to remove special cases.
public static void WriteToFile(Customer customer, string path)
{
// Write the customer to the file handling all subclasses correctly.
// ...
}
If you still don't have the time or it's just not feasible, you can sometimes constrain the arguments the method will take to be more specific.
public static void WriteToFile(SpecialCustomer customer, string path)
{
// Look closely - I've reversed the special-ness of the handling!
// ...
}
Interestingly, the exact example I showed before won't work - we wanted to avoid a special case and handle a more general one; instead the problem with over-promising is that the API seems to handle a general case but only handles a special one.
The problem with this API actually needs to be solved elsewhere: why can't SpecialCustomer behave like a Customer? Assuming SpecialCustomer inherits from Customer, it would be violating the Liskov substitution principle, so looking at the classes under this lens and trying to fix this would probably be a better approach.
Another problem with this particular API is that the functionality is outside the class, so it's not clear how it will handle more-derived types if there is a meaningful way in which that should work: will properties from SpecialCustomer be written out or not? A much better approach would be to have this method exist on the Customer class, and then SpecialCustomer could specialize that as needed.
As a last resort for most lying APIs, you can usually change the names to be more descriptive of whatever you would have lied about. This is typically a last-recourse solution when there are other options, because it's usually leaving the underlying problem unaddressed, and it leaves the user wondering whether there is another API it should go to that works better, or whether this is some optimized special case, or why they would see such an API in the first place. Confusing someone is better than lying to them, but not by much.
Enjoy!