I wanted to briefly comment on something I’ve observed myself doing that I consider to be an antipattern.
Let’s say I’m knee-deep in that swampy mire known as Someone Else’s Code. I’m working on a search service that has this API:
class SearchService(object): def search(self, query): # Mega complicated stuff follows.
I need to change this method to be aware of the content-type of the media it returns.
OK, let’s go see how often this thing is used.
~$ ack search_service.search <.. SPEW of 86 distinct invocations of this service method>
I pick one of those service calls and crack open open the relevant buffer:
def viewThatHasNoContentTypeInfoWhatsoever(request, query): context.search_service.search(query)
Even better — now I have to figure out how I’m going to get the content-type information I need to the service I’m calling here.
And there are 86 more of these to go.
So now I’m sitting here thinking that the job I initially thought was going to take 5 minutes is probably going to take much longer.
So what are my options?
content_typea required arg on the search method and just deal with the upstream changes.
- Refactor the parameter list to search to take a
SearchParamsobject that encapsulates the parameters that are required for a search.
- Create a different, content-type-aware method on
SearchService, since this is a conceptually different search, and use that instead.
1 and 2 both require changes to all 86 call sites, but at least 2 is a bit more future-proof. 3 just seems like it’s asking for an explosion in combinatorial complexity on that service interface.
This is generally the point where I begin to consider option 4, which is to make
content_type a keyword arg with a reasonable default.
Great, right? This is a “minimally disruptive change” — I’m guaranteed not to break any client code, which is good because our test coverage is low and I’m not confident that I’ve even found all the relevant call sites.
The biggest problem I have with this is that it’s a terrible idea.
If this feature was being designed for the first time right now, I’d probably decide there there is no “reasonable default” for
content_type at all, and I’m thus fragmenting the interface to the call at the intersection of version n-1 and version n.
New developers on the team aren’t necessarily going to be aware of the entire release history of the project, and so seeing
media_type as an optional parameter will suggest to them that this is just a minor detail of the interface, where it is actually just as important as the querystring itself is.
I wonder — do you ever catch yourself being similarly lazy? I feel like this technique might actually be a good first step in getting hooks into legacy code for some preliminary unit testing before elevating the parameter to first-class status and making the upstream changes, but that’s only valid if you’re committed to making those changes in the first place.
In the meantime, I’ve forced myself to assess my motives carefully each time I add an optional kwarg to a function or method in Python. (Constant vigilance!)