Better conditional statements

I don’t know if you are like me, but I really have a hard time reading complex ‘if’ statements with boolean expressions.

For example:

if ("male".equals(person.getGender()) && person.getAge()>=18 && !"french".equals(person.getNationality()) ||
 "female".equals(person.getGender()) && person.getAge()<18 && !"german".equals(person.getNationality()) {
 // do something
 }

If you are like me, my recommendation is to give explicit names to each expression, like this:

boolean isNonFrenchMaleAdult = "male".equals(person.getGender()) && person.getAge()>=18 && !"french".equals(person.getNationality());
boolean isNonGermanFemaleChild = "female".equals(person.getGender()) && person.getAge()<18 && !"german".equals(person.getNationality());

 if (isNonFrenchMaleAdult || isNonGermanFemaleChild) {
 // do something
 }

From there, it might also be interesting to make the conditional expressions part of the Person class to end up with:

if (person.isNonFrenchMaleAdult() || person.isNonGermanFemaleChild()) {
 // do something
 }

You can even go further and combine the 2 methods in a single one:

if (person.isEligibleToDoSomethingVeryInteresting()) {
 // do something
 }

Finally, why not completely move all the logic inside the Person class ?

person.doIt();

Note that in practice, you may not want to move all the logic inside the Person class.

For example, if the ‘if’ statement persists the Person instance in a database, you may not want to put the data access logic in the domain object.

Laurent KUBASKI

Advertisements

About lkubaski
www.kubaski.com

6 Responses to Better conditional statements

  1. Sam says:

    Like the idea! its a good one

  2. Frisian says:

    I think your missing two points here:
    1. What does the last if-statement logically stand for (non-french male adult or non-german female child)? Maybe a special offer or something like that? That should be combined into one method like isEligibleForSpecialOffer(), so when you extend the person class, you know why and how to change it, so depending code continues to work fine.
    2. The code inside the if-statement should be interned to the person class, so you can say person.doSomething(). When it comes to OO, it should be “tell, don’t ask”.

    • lkubaski says:

      #1: I agree with you: both methods could be combined into one in the context of this simple example. This would not be the case if isNonFrenchMaleAdult() was reused elsewhere (and to be honest, this was the case in the project where the example was extracted from).

      #2: In the ideal “everything is OO” world maybe, but in practice, you can’t always (or simple do not want to) move all the logic inside the class it theoretically belongs to. For example, if the ‘if’ statement persists the Person instance in a database, you may not want to put the data access logic in the domain object.

  3. Frisian says:

    #1: Even when isNonFrenchMaleAdult() is used elsewhere, I’d put the combined condition into a method of its own. It avoids logic scattered all around the code and hence gives you tighter control and a higher level of abstraction. This level of abstraction shoud ideally be reflected in the naming.
    #2: Especially, if you don’t want to put everything in you person class (think Visitor pattern), centralizing logic into one place becomes even móre important.

    • lkubaski says:

      Well, the problem is that when you try to illustrate your point with a simple example, there is ALWAYS someone talking about applying design patterns 😉

      In fact, I’m surprised that no one said that having the getGender() return a String is bad practice and that it should return an enum instead !

      My point here was only to talk about these long ‘if’ statements that you often see and to explain how to make them more easily readable. Introducting a intermediate boolean value is easy… replacing this with the Visitor design pattern is not.

  4. lkubaski says:

    I’ve added your two suggestions to the post… but I think it’s worth mentioning that moving all the business logic in the Person class may not be the best option.

    Thanks for your comments !

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: