One of my personal pet peeves is code that contains a bunch of conditional logic to handle seemingly special cases that really aren’t.
For example, let’s say we have a page that shows a user their playlists if they are logged in, but this page also has other useful information on it, and users aren’t required to log in. In our original design for our authentication class, if authentication “failed” (i.e. the user was not logged in), a call to Auth::getUserID() would return an empty string.
With that implementation, every piece that might have something to display to a logged in user has to have a bunch of conditional logic checking to see if the user is logged in, doing one thing i they are and another if they are not, adding unnecessary complexity and, of course, more potential for bugs to crop up.
This type of logic is completely unnecessary. I changed Auth to return a userID of 0 if the user is not logged in, and now it is not necessary to have any special handling for that case. If the user is logged in, they get playlists. If the user is not logged in, userID 0 does not have any playlists so they do not get playlists. I estimate that this simple change made over 100 lines of code obsolete. Not a whole lot of code in the grand scheme of things, but how many bugs can hide in 100 lines of code?
Another example of the same principle involves exceptions (which, as regular readers of Raymond Chen know, are hard to deal with). Under certain circumstances, recommendations from an external source can be included with our normal set of recommendations. The author of the recommendation-supplementing code originally had it throwing exceptions whenever it had problems. Of course, the case of not having supplemental recommendations because of an error is not really a special case. As far as my code is concerned, you just don’t have supplemental recommendations. In this case modifying the original code eliminates unnecessary handling for a special case and eliminates the potential for an uncaught exception to slip through.
A general rule of thumb to help avoid unnecessary special cases, at least with PHP, is to always return what you say you are going to return (and don’t throw exceptions). If your method processes some data which results in an array, return an array even if the processing has no results. This is the way most native methods in PHP already work, if you think about it. count(array()) doesn’t return an empty string or null or raise an exception, nor does count(null). Really, neither of these are special cases. In either case, the number of elements is zero, and developers are not required to care about the differences.