Okay, here's a breakdown of the next rule:
Don't use the else keyword.
Although there is nothing wrong with the else keyword itself, there are two forms of abuse that drastically reduce your code readability. The first is too much code in each of the branches which makes it hard to determine where you are in your branches. I remember when I first started coding I found that once I scrolled to the next page after the initial if / else I had a hard time determining whether the closing brace I was looking at belonged to a for loop, an if block or what. My solution at the time was to put little comments at the end of the braces like this:
It didn't seem so bad to me at the time since I was also playing around with bash a lot and it kinda looked the same. After a while though I got sick of typing them. I think the real reason I stopped adding them was that I learned to take the pages of code and turn them into functions so that the "end" comments seemed silly.
If you get lost trying to follow the branches in that chunk of code, you have chosen the wrong career. The end comments were dropped. For simple if/else conditions, I now choose to set a default and use return to skip processing more than I should. So instead of something like this:
I would now drop the else entirely:
Could just be a style thing, but the second version seems like so much less work to read... *shrug*
The other way that if/else blocks can get nasty is if you have crazy nesting action. Of course you should never have that situation if you followed the
first rule. hehe. One of the key ways to handle logic in a more OO way is to employ the strategy pattern. I know, I know. Patterns are the devil! Patterns are for n00bs! Would you prefer that I say
you should use polymorphism to change the internal implementation of a method at run-time? Seriously though, get over yourself.
So let's say that we are going to process credit cards, there is special handling for each of the card types we want to process: Amex, Visa and Mastercard. Here is some code that might do this:
Believe it or not, I have actually seen this code out in the real world. It is definitely hard on the eyes, and it's only got three credit card types in it. The original code also processed Diners, Discover, JCB and EnRoute. I especially love the mixed switch and if action. So sexy! Here's what the code *could* have looked like if the author had broken out each strategy for determining the card type based on the card type itself:
I am not going to show you the details of the *CreditCardType classes as they should be pretty trivial to write. Just take all of the junk related to validating each card type and stick it in the matches method. So simple! Although the code in the getByCardNumber method is much simpler (almost 1/3 the size) it can easily be extended to handle any number of card types by simple appending the new card type to the array of names and by adding the appropriate type class. Believe it or not I actually rewrote this code after my first draft to make it more English-like. Line #8 of the last example used to read "if ($validator->validates()) {" - whatever that means. Heck if you really wanted to get fancy you could use a directory iterator and not bother with a hard-coded list at all. Long story short, try to keep your logic branching from getting out of control. It doesn't do anything to make your code better.
Next post will be rule #3:
Wrap all primitives and strings.
