While learning WPF I encountered this example on Microsoft’s website. You can either scroll down or download the C# version of the code.
It’s about this code:
The entire inner if statement can be reduced to:
This reads very well in plain english (unlike the original version which is
“accepted means that the product’s current price is smaller than 25”.
Actually, the whole method can be rewritten as:
This is only 5 lines of code, unlike the original version which is 16 lines of code, 11 lines of code waste! The final is just one third! Imagine if this would be happening in a real project!
I really hate when people don’t understand boolean logic and write billions of lines of code that others have to read and maintain!
Regarding the example above: I would go even further. If we look at the method
name we see that it shows bargains. This concept can be expressed more
clearly in the code by creating a function
This is called the tell, don’t ask! principle.
Here’s how I would do it:
This type of logic should be in the domain model, not in the user interface as it was in the original code.
Dealing with boolean expressions
Here are some of the bad examples I encountered and how you can correct them. The examples are in java, but it shouldn’t matter.
Don’t compare a boolean with true! It is already true!
Same goes for false:
Other programmers have an urge to check the boolean when returning from a function.
Some other programmers use the ternary operator:
And finally: don’t EVER use booleans as parameters to a function. This violates the Single Responsibility Principle. If you need a boolean parameter you are doing 2 things: one for true and the other for false. If you didn’t know about the Single Responsibility Principle, maybe you should check the full stack which is called the SOLID principles.
Here are some examples:
Tips and Tricks
How can you avoid writing code as in the original example? Well, here are some tips:
- use guard clauses,
- remove stupid comments that just repeat code,
- remember that the “stuff” you put between parens is a boolean expression and that you can assign it directly to a variable…
- remove all those stupid brackets when you have single statements. And you SHOULD always have only one statement (extract functions for each branch)
The moral of the story
The C# code I showed in the beginning of the article is an example, a demo… no wonder why new programmers (and not only) write shitty code! When they start learning a new technology they only see piles of crap all over the Internet.
So, one moral is for software professionals to be more careful what type of code they use as examples (unlike Microsoft in this case).
What type of code YOU want to promote? Don’t forget that it always backfires and you end up working with people that will write the same kind of code.