Approaches for not complex conditions

Question

I encountered code like this below. I was told its fine, and boilerplate is not always better. I agree boilerplate might be bad, but I am not sure about something like this:

    if (entity.isSomething && wasCheckedBefore(entity.name)) showSomeMessageToUser(entity.name) 
    if (entity.isSomethinElse && wasCheckedBefore(entity.name)) showSomeMessageToUser(entity.name) 
    if (entity.isSomethingSomethinElse && wasCheckedBefore(entity.name)) showSomeMessageToUser(entity.name) 

What would you recommend? (rather not CoR or Command pattern)

//Edit I understand what do you mean by saying this code it's fine. What if there were more conditions etc? Whats the best way to go?

Answer 1

I'll be honest, I don't see a whole lot wrong with the code sample you listed--assuming it's just a handful of cases like that. There are a few ways you can change the code that may or may not make the resulting code more readable or the intent more clear.

One example is to separate the common clause in the if statement from the rest of the code like this:

if (wasCheckedBefore(entity.name)) {
    if (entity.isSomething) showSomeMessageToUser(entity.name) 
    if (entity.isSomethingElse) showSomeMessageToUser(entity.name) 
    if (entity.isSomethingSomethinElse) showSomeMessageToUser(entity.name) 
}

Another option is to have an enum type parameter to do a switch statement:

if (not wasCheckedBefore(entity.name)) return

switch(entity.type) {
    case type.something:
        showSomeMessageToUser(entity.name)
    case type.somethingElse:
        showSomeMessageToUser(entity.name)
    case type.somethingSomethingElse:
        showSomeMessageToUser(entity.name)
}

Or extending that concept you can simply pass the type into the method so it really becomes shorter and the complexity moved to the showSomeMessageToUser function:

if (wasCheckedBefore(entity.name)) {
    showSomeMessageToUser(entity.type, entity.name)
}

The bottom line is that none of this is functionally any better than the other. It's just shuffling the logic around. The question is whether any of these approaches make the intent of the code more prevalent. If not, let sleeping dogs lie.

Answer 2

If there's a good domain term for what's being checked, you can convert each boolean expression to a method:

// This method name should explain what the boolean expression 
// means in terms of domain logic, i.e. why it is important
// to check both of these values.
private boolean someDomainConcept(entity) {
   return entity.isSomething && wasCheckedBefore(entity.name)
}

if (someDomainConcept(entity.name)) showSomeMessageToUser(entity.name);

This can add some documentation value to the code while making it a little more brief to use your boolean expressions and enabling reuse of those expressions. I wouldn't say that this is always a valuable pattern; it depends on your actual use case. Sometimes there isn't a better name for the boolean expression than just the sum of its parts.

Answer 3

Why don't you just consolidate your messages inside the entities?

class SomethingEntity
{
    string name;
    bool isChecked;

    public string Message()
    {
        if (isChecked)
            return someMessage;
        else
            return someOtherMessage;
    }
}

You can then

showMessageToUser(anyEntity.Message());

There are many ways to skin this cat. You probably don't need a well-known software pattern.