Thursday, May 28, 2015

Beware of "Never" and "Always"

If I hear someone say "Always do this" or "Never do that", I generally run the other way (but there are exceptions, of course ☺).

I am a big fan of best practices. The reason for this is that I spent a lot of time in my career doing things in a "could be a lot better" way and finding out the hard way that my approach had some issues that I didn't think about.

Eventually, I came to my senses and started looking at recommendations from people who have been programming way longer than I have. They've already made those mistakes and come up with better solutions. These get described with the general term "best practices".

A Good Place to Start
I treat advice that I get from other developers (and the developer gurus) as a good place to start. This means that I will generally follow the advice unless I come up with a really good reason not to.

Here's an example that I like to show in my presentations:


"Hikers and bikers move to the side of the road when a vehicle approaches." This sounds like a great piece of advice. We don't want to get run over. But we also need to consider our surroundings.

But Don't Follow Blindly
Sometimes when we look at our surroundings, we find out that following the best practice may not be a good idea after all:


So even though following this advice is generally a good idea, it would actually cause us more problems in this particular environment. I'd prefer to take my chances with the cars than the alligators.

BTW, I found out that this picture is from Shark Valley -- part of the Everglades National Park in Florida. (And I'd love a better quality image of this if anyone has one.)

User Input is Evil
Here's something we all need to be aware of:
Fact: All input is evil.
I don't think anyone disagrees with this. Anyone who has an open form on a website knows this first hand. And if you don't want to believe me, just ask little Bobby Tables (xkcd: Exploits of a Mom):


And this leads us to a good piece of advice:
Best Practice: Verify input before using it.
It's really hard to argue with this. When we're talking about input, we're not just referring to things that users type in, we're also referring to input that we get from other developers when we create APIs or library methods.

Implementing a Best Practice...
I've spent several articles exploring guard clauses on a particular method. This is the method that we've been looking at when exploring TDD and Conway's Game of Life. Here's the original method:


We don't validate the parameters. And we saw why this was a problem in the last article: Testing for Exceptions with NUnit. It's possible to pass in an invalid "currentState" parameter and get a seemingly-valid value out of the method.

At the same time, the "liveNeighbors" parameter is really only valid for values between 0 and 8 -- the number of neighbors that a square-shaped item can have in a grid.

So, we spent some time adding guard clauses at the top of the method:


And we also saw a couple different ways to write unit tests to make sure these guard clauses work as expected: both using NUnit and letting Smart Unit Tests check them.

But after going through the exercise of adding the guard clauses and creating tests for them, I promptly removed them.

...And Then Ignoring It
Yes, I just admitted to actively ignoring a best practice of validating input. But there is a specific reason for that. In the application where this method is used, speed is important. This method gets called thousands of times for each generation when we run our Conway's Game of Life application. And we could end up running through hundreds of generations during that process. That means this method gets called hundreds of thousands of times.

In fact, in an effort to parallelize the method calls, I ran through several experiments to see what is most efficient: Optimizing Conway's Game of Life.

Here's the method (and level of parallelism) that worked best for this scenario:


The "GetNewState" method (the one we've been looking at) is called in a nested "for" loop. And because this method is called so often, we need it to run quickly.

Guard Clauses Slow Things Down
To keep this method running quickly, we want to trim as much code as possible. The guard clauses may not seem like they add a lot of work (just 2 "if" conditions), we see a big difference if we actually check some metrics.

Here's the performance of our method *without* the guard clauses (the same as we saw in the optimization article):


The 10,000 iterations is the number of times the entire grid is updated. Since we have a 25x70 grid, each iteration runs this method 1,750 times. And we can see our result is about 1-1/2 seconds.

Here's the performance with our guard clauses added:


This is 4 times slower -- definitely not acceptable when speed is an important feature.

Have a Good Reason
I recommend best practices to developers as much as possible. What I've found is that they are good advice to follow the vast majority of times (probably in the 95% range). It's really those exceptional cases (like when alligators are resting on the side of the road) that we decide to ignore the practice.

But we're not really ignoring the best practice at that point. "Ignore" implies that we don't care about it. We do care about it, but we've determined that it's not appropriate for this particular scenario. And we also understand the consequences of our decision.

In our example, I understand the consequence of not having the guard clauses: it is possible for someone to call this method with invalid parameters, and the method will return an invalid response. But *in this scenario*, this method is not published to the outside world. Instead, it is called by another class in the application (which I have full control over). And the way that class is set up, I know that the method will not be called with invalid parameters.

So I understand the risk, and I'm willing to take that on because performance is a very high priority here.

Best Practices are Awesome
I really like learning about best practices. I want to understand the problems that other developers have run into. I want to understand the solutions that they have come up with. And I want to steal as much of that experience as possible so that I can skip over the painful part of the process.

And that's why I like to show best practices to other folks, whether it has to do with abstraction, dependency injection, design patterns, or general coding style. They can save us all sorts of time. But we shouldn't follow them blindly. We should take time to understand the benefits and consequences. This way, we know that we're using the right tool for the job.

So rather than thinking of these as "always" or "never" items, we need to think of them as a great place to start. Most of the time, they are the right answer. But we have the option of choosing another path if that's what makes most sense.

Happy Coding!

No comments:

Post a Comment