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:

// MainWindow.xaml.cs
private void ShowOnlyBargainsFilter(object sender, FilterEventArgs e)
{
    AuctionItem product = e.Item as AuctionItem;
    if (product != null)
    {
        // Filter out products with price 25 or above
        if (product.CurrentPrice < 25)
        {
            e.Accepted = true;
        }
        else
        {
            e.Accepted = false;
        }
    }
}

The entire inner if statement can be reduced to:

e.Accepted = product.CurrentPrice < 25;

This reads very well in plain english (unlike the original version which is also longer):
“accepted means that the product’s current price is smaller than 25”.

Actually, the whole method can be rewritten as:

// MainWindow.xaml.cs
private void ShowOnlyBargainsFilter(object sender, FilterEventArgs e)
{
    AuctionItem product = e.Item as AuctionItem;
    if (product != null) e.Accepted = product.CurrentPrice < 25;
}

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 isBargain in AuctionItem.
This is called the tell, don’t ask! principle.

Here’s how I would do it:

// MainWindow.xaml.cs
private void ShowOnlyBargainsFilter(object sender, FilterEventArgs e)
{
    AuctionItem product = e.Item as AuctionItem;
    if (product != null) e.Accepted = product.IsBargain();
}

// AuctionItem.cs
public bool IsBargain()
{
    return CurrentPrice < 25;
}

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!

// bad way
if (something == true) doThis();

// good way
if (something) doThis();

// even better: rename the boolean so it reads like English
if (isSomething) doThis();

// good examples: isBargain, isValid, isFile, exists, shouldReceiveBonus

Same goes for false:

// bad way
if (something == false) doThat();

// good way
if (!something) doThat();

Other programmers have an urge to check the boolean when returning from a function.

// bad way
if (condition)
    return true;
else
    return false;

// good way
return condition;

Some other programmers use the ternary operator:

// bad way
boolean active = userDisabled? false : true;

// good way
boolean active = !userDisabled;

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:

// bad way
setVisible(true);
setVisible(false);

// good way
show();
hide();

// other good examples: enable/disable, switchOn/switchOff

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.