CodeSOD: Comments, Documentation, and Nulls
Ah, the joy of comments. Good comments can illuminate complicated code, explain a programmer's reasoning, or even just do their best to absolve a burned out programmer of their sins. "Yes, it's bad, but it works."
Then there's this comment, sent to us by Mark B. This is from a configuration file format, "helpfully" explaining what the flag does.
// [true, false] if this is true the provided credentials will be encrypted i.e. the provided credentials will not be encrypted
Now, the developer clearly meant "if true, we encrypt, if false, we don't", but clearly didn't understand what "i.e." means ("that means", or "that is to say", or "in other words"), creating a WTF for grammarians everywhere, and a headscratcher for the developer reading this for the first time.
While we're picking on documentation, let's pick on someone our own size, or maybe a little larger. Let's pick on Adobe. Nancy recently had to use Adobe's analytics system, which they describe thus: "Everything about this API is terrible."
And there are choices in that API. Instead of using arrays, for example, the tracked events are placed into a data structure like this: _experience.analytics.event1to100.event33. "This pattern repeats every 100 events to _experience.analytics.event901to1000.901-_experience.analytics.event901to1000.event1000.
Now, I'm sure this wasn't a choice made lightly, and probably is coupled to some internal implementation that makes arrays a non-starter for this field. But it's still the kind of API that makes me cringe. Especially as the API does have fields, like productListItems, that are arrays.
And finally, let's close out today's installment with some actual code. James sends us this one-liner from the WHERE clause of a stored procedure:
emp.LawbaseCode = ISNULL(null, emp.LawbaseCode)
The ISNULL function takes two parameters: a check expression which may be null, and a value to return if that expression is null. Since null is always null, this will always return emp.LawbaseCode, which is also the value we're comparing against, making this a check to see that a field equals itself within a WHERE clause.
James writes:
[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!Not sure how it got there or what it was even supposed to be doing. At best it's a great way to reduce query performance.