Get Off My Lawn
Get Off My Lawn

Reputation: 36311

Shorten an if statement with lots of || operators

Is there a way to do an if so I don't have to do lots of || operations?

So is there anyway to shorten this to make it easier to read?

if(t.triggerOn == TriggerOn.CreationDelay || t.triggerOn == TriggerOn.CollisionDelay || t.triggerOn == TriggerOn.RigidbodyRestDelay || t.triggerOn == TriggerOn.LayerCollisionDelay || t.triggerOn == TriggerOn.EnterRadiusDelay){
    // Do something
}

TriggerOn is an enum

Upvotes: 0

Views: 146

Answers (5)

Chris Simmons
Chris Simmons

Reputation: 55

I believe in this case for readability you may want to consider a switch statement.

switch (caseSwitch)
{
    case TriggerOn.CreationDelay:
        //do some stuff
        break;
    case TriggerOn.CollisionDelay:
        //do more stuff
        break;
    default:
        //do some more stuff
        break;
}

or with multiple case for one decision

switch (value)
{
   case TriggerOn.CreationDelay:
   case TriggerOn.CollisionDelay:
   case TriggerOn.RigidbodyRestDelay:
      //do some stuff
      break;
   case TriggerOn.LayerCollisionDelay:
   case TriggerOn.EnterRadiusDelay:
      //do more stuff
      break;
   default:
       //default stuff
      break;
}

Upvotes: 1

David Hiblen
David Hiblen

Reputation: 271

If you have control of the enum, then decorating with attributes can give nice readability and separate the metadata from your code. For example:

public enum TriggerOn
{
    [DelayedTrigger]
    CreationDelay,

    NoCreation,

    [DelayedTrigger]
    CollisionDelay,

    CollisionStandard,

    [DelayedTrigger]
    RigidbodyRestDelay,

    [DelayedTrigger]
    LayerCollisionDelay,

    [DelayedTrigger]
    EnterRadiusDelay
}

public class DelayedTriggerAttribute : Attribute
{

}

You can then re-write your code to use a nice extension method which tests for the attribute, e.g.:

t.triggerOn.IsDelayedTrigger()

Upvotes: 0

Get Off My Lawn
Get Off My Lawn

Reputation: 36311

I did create an IN method that you can search an enum, but it doesn't really save much space.

public static class Search<T> {
    public static bool IN(T val, params T[] items){
        foreach (T item in items) {
            if(val.Equals(item)){
                return true;
            }
        }
        return false;
    }
}

It gets used like this:

if(Search<TriggerOn>.IN(t.triggerOn, TriggerOn.LayerCollision, TriggerOn.EnterRadius...)){
    // Do something
}

Upvotes: 0

AlexCharizamhard
AlexCharizamhard

Reputation: 377

Something like this could work... if you are checking all delays.

var _delaylist = TriggerOn.GetType().GetProperties().Where(x => x.Name.Contains("Delay"));

if (_delaylist.Any(x => x.GetValue(x) == t.TriggerOn)
{
    //dowork
}

Upvotes: 1

andeart
andeart

Reputation: 951

If you really want to reduce the number of characters, you can use the indices of the enum-values. Example:

    if(t.triggerOn == 0 || t.triggerOn == 1 || t.triggerOn == 2 /* ... */)
    {
        // Do something
    }

where 0, 1, etc. must correspond to the indices of the TriggerOn values you want. The drawback to this is, it's hard to tell what conditions you're accepting just by looking at the line.

That said, you could also break your original code into multiple lines for readability.

    if(t.triggerOn == TriggerOn.CreationDelay ||
       t.triggerOn == TriggerOn.CollisionDelay ||
       t.triggerOn == TriggerOn.RigidbodyRestDelay ||
       t.triggerOn == TriggerOn.LayerCollisionDelay ||
       t.triggerOn == TriggerOn.EnterRadiusDelay)
    {
        // Do something
    }

I would suggest to keep your code as is (maybe break it into multiple lines), because understandability is readability.

Understandability... Hmmm...

Anyhow, I hope that helps!

Upvotes: 2

Related Questions