Though so far I hate all the forced unwrapping .NET requires with nullables now -- which was stolen directly from Swift -- this comment on a Skeet answer explaining them a bit, authored by someone you should recognize if you can spell C#, includes one of the most important concepts I've been trying to PR into coworkers for quite a while. Not sure I've ever said it this eloquently, though, so...

Here's that key point from Eric Lippert (emphasis mine):

  • The key thing that I learned when studying the Coverity checker is that code isย evidence of the beliefs of its authors. When we see a null check that should inform us that the authors of the code believed the check was necessary. The checker is actually looking forย evidence that the authors' beliefs were inconsistentย because it is the places where we see inconsistent beliefs about, say, nullity, that bugs happen.ย 

(This might be that Coverity stuff he's talking about. Not sure.)

One place I run into disconnects between code and intent routinely is with Jasmine tests. If tests essentially share the exact same code across Arrange sections, it should be in a shared describe block with a beforeEach DRYing up all that setup. If you only change the arrange on tests by something trivial, those tests too should share an beforeEach block with just the differences in the individual tests' Arrange sections. Otherwise I'm trolling through each arrange block trying to figure out what's different.

(Actually I expect a method even for the slightly different ones -- especially for the slightly different ones, because then you can have parameters communicating what's different in the calls out. Make sense? It's all about the DRYing, folks.)

Tell me what the tests do by grouping same and different, keeping things DRY so I can get familiar with concepts you're using over and over -- in ways I can't when you just cut and paste code all over the place.

Put more succinctly, I hate cut and pasted code because I expect code blocks that seem similar to be slightly different. Otherwise why not DRY them? Because unDRYed code means we've got some code blocks that are exactly the same mixed with some that aren't with no easy, scannable way to tell which is which.

Another domain that suffers is checking for strict equality against undefined or another falsy value in JavaScript. If I see if (myVar === undefined) that tells me null and 0 and any other falsy value is okay and different from undefined. Even if the value should be undefined at this point, if its possible non-undefined value is, say, an object literal, then there's no reason not to also exclude anything falsy.

That is, unless it makes the code worse, follow conventions! When we're in JavaScript land, because it's a dynamic language, I always assume we prefer operating with a truthy/falsy mindset. If you're comparing to a specific value (or !!ing a Boolean to cast it to a strongly typed bool), I assume you have a good reason to use an anti-pattern, like with myVar || myVar === 0 (though consider parseInt(myVar, 10) (or float) instead?). If I'm wading through code to see a null for an object literal would also be bad news, I've wasted time. You didn't write what you meant.

If your anti-patterns aren't "evidence of your beliefs", we're doing it wrong! And if we don't share patterns, we should talk about it until we do -- or at least understand each other's dialects.

/rant

Labels: ,