3

Is there a shorter version of IF statement to do this?

if (el.type == ElementType.Type1 || el.type == ElementType.Type2)
David
  • 208,112
  • 36
  • 198
  • 279
Gapipro
  • 1,913
  • 2
  • 22
  • 34
  • 3
    This is not "optimization"... And, just out of curiosity, if you could design a shorter version to look like *anything that you wanted*, what would it be? I have a hard time imagining *how* that could get any shorter without losing its content. – Cody Gray - on strike Dec 09 '10 at 14:06
  • 4
    Why? It's perfectly plain and easy to understand as it is. Why obfuscate it to save a few bytes of pre-compiled code? – David Dec 09 '10 at 14:07
  • @Cody Gray: my guess is that Gapipro would want it to look like this: if (el.type == (ElementType.Type1 || ElementType.Type2)) – Kristof Claes Dec 09 '10 at 14:09
  • Just about the only way to make it shorter would be to move the logic somewhere else, like an extension method... – cdhowie Dec 09 '10 at 14:09
  • Can you clarify whet you mean by shorter? What would you like to write? – rds Dec 09 '10 at 14:18
  • @Kristof Claes: Yes something like that... – Gapipro Dec 09 '10 at 14:29
  • @Gapipro: Unfortunately, that syntax doesn't mean what it appears to mean at first glance. – cdhowie Dec 09 '10 at 14:42
  • @cdhowie: I know, that's why I asked this question if there is something like that... – Gapipro Dec 09 '10 at 14:47
  • @Gapipro: There's not, sorry... – cdhowie Dec 09 '10 at 14:47
  • I think, more importantly than the fact that Kristof's proposed syntax doesn't mean the same thing, is that it *already means something else*. And while I agree that might be a little shorter, it would put us into the difficult situation of having to come up with a new way of expressing what that already means. And so on, like a snowball rolling down a hill. – Cody Gray - on strike Dec 09 '10 at 15:18

13 Answers13

9

You could use an extension method, but would this really be much better?

Throw this on a static class:

public static bool IsOneOf(this ElementType self, params ElementType[] options)
{
    return options.Contains(self);
}

And then you can do:

if (el.type.IsOneOf(ElementType.Type1, ElementType.Type2)) {

However, this will be a lot slower than your if statement, as there is an implicit array initialization followed by an array traversal, as opposed to (at the most) two compares and branches.

cdhowie
  • 158,093
  • 24
  • 286
  • 300
1

Consider ElementType is defined as

enum ElementType
{
Type1,
Type2,
Type3
}

In this particular case you may write if(el.type<ElementType3) By default Type1 equals to 0, Type2 equals 1, etc

Tony Kh
  • 1,512
  • 11
  • 8
  • 7
    You may, but please don't. – Dave Markle Dec 09 '10 at 14:08
  • Can go wrong very fast. A workaround could be having the enums with fixed values (Type1 = 1; Type2 = 2; Type3 = 3). It would a bit safer, but still misses the point of using enums instead of integer constants – t3mujin Dec 09 '10 at 16:20
1

If you have only 2 values, I strongly suggest to use the code you posted, because is likely the most readable, elegant and fast code possible (IMHO).

But if you have more cases like that and more complicated, you could think to use a switch statement:

switch (el.type)
{
    case ElementType.Type1:
    case ElementType.Type2:
    case ElementType.Type3:
        //code here
        break;
    case ElementType.Type4:
    case ElementType.Type5:
        //code here
        break;
    case ElementType.Type6:
        //code here
        break;
}

that translated in if statements would be:

if (el.type == ElementType.Type1 ||
    el.type == ElementType.Type2 ||
    el.type == ElementType.Type3 )
{
    // code here
}else if(el.type == ElementType.Type4 ||
         el.type == ElementType.Type5)
{
    // code here
}else if(el.type == ElementType.Type6)
{
    // code here
}

They're perfectly equal to me, but the switch seems more readable/clearer, and you need to type less (i.e. it's "shorter" in term of code length) :)

digEmAll
  • 56,430
  • 9
  • 115
  • 140
0

You can try this:

if(new [] { ElementType.Type1, ElementType.Type2 }.Contains(el.type))

(turns out, that takes even more characters)

Anton Gogolev
  • 113,561
  • 39
  • 200
  • 288
0

I guess you're referring to an IN() clause or some such? Not really... Well, sort of... You can do something like:

if ((new [] { ElementType.Type1, ElementType.Type2}).Contains(el.type)) {...}

But that's not going to be anywhere near as performant (or brief) as what you're already doing. You can also do

if (el.type == ElementType.Type1 | el.type == ElementType.Type2)

but that doesn't do short-circuit evaluation, so you rarely want to use that operator. My advice is to stick with what you have.

Dave Markle
  • 95,573
  • 20
  • 147
  • 170
  • 2
    The thought of omitting the second `|` in order to reduce the character count by 1 while sacrificing the performance benefits of short-circuiting makes me laugh. And then cringe. – Cody Gray - on strike Dec 09 '10 at 14:13
  • 1
    Ha! Hey, he asked how to make it shorter... ;-) I use it so infrequently that my feeble brain only thinks of it as a bitwise operator. – Dave Markle Dec 09 '10 at 14:19
0

The brief answer is no. There isn't C# language construct that lets you combine object comparisons. But as many people have mentioned before, creating a collection of your types is probably your best bet in creating a shorter if statement. However that sacrifices quite a bit in the area of performance. I would stick with the OR statement.

Wes P
  • 9,622
  • 14
  • 41
  • 48
0

There is no better way to optimize your code. As other users have shown, you can optimize an if else.

But a type of if statement I have thought about, in your case especially, would be

if(X > [Y || Z || A])

But that doesn't exist, and isn't as clean as the current if (X > Y || X > Z || X > A)

(This is more of a response to Cody Gray)

Anathema
  • 5
  • 1
0

In short: nothing reasonable (reasonable in terms of code readability and performance optimisation). I wouldn't recommend the ternary operator for this kind of comparison either.

The actual if can be shortened to 5 characters ;)

bool b = (el.type == ElementType.Type1) | (el.type == ElementType.Type2);

if(b){...}
Jules
  • 1,352
  • 7
  • 19
  • 2
    Do you have particular basis for the claim that inequality tests (`!=`) are faster than equality tests (`==`)? – Cody Gray - on strike Dec 09 '10 at 14:21
  • My apologies, that was rubbish. I just mixed up some stuff I remembered from early Uni ASM and CPU design days. Edited it. I did a quick research again, and it actually doesn't matter if the value to compare fits wholly in one register. Additionally, I learnt that depending on the compiler used, equality can even compare faster on some high level languages. – Jules Dec 09 '10 at 14:35
  • Yeah, that's because lots of high level languages often define inequality in terms of equality, forcing them to determine equality first. I was just curious if you were thinking of some other language I don't know personally. The real point is that, in the end, the difference is negligible. (And for what it's worth, I think you're still right about any difference being optimized out by the compiler picking the optimal instruction (or, certainly by the JITer in C#). – Cody Gray - on strike Dec 09 '10 at 15:24
0

If this is a common logic comparison in your code that shows up alot I'd just write a method to handle it.

private bool isType1OrType2(ElementType type)
{
    return type == ElementType.Type1 || type == ElementType.Type2;
}

then you can do

if(isType1OrType2(el.type))

You could also make this an extension method like so

public static bool isType1OrType2(this ElementType type)
{
    return type == ElementType.Type1 || type == ElementType.Type2;
}

so the code would read a little nicer

if(el.type.isType1OrType2())

But then you have to have a static class but you can decide if it's worth it. I personally would not write a method to take a collection of types to compare to unless you find that you are comparing the type to many different combinations. I also would not even bother changing the code at all if this is the only place you make this type of comparison.

juharr
  • 31,741
  • 4
  • 58
  • 93
0

i dont think there is a way to optimize your statement

Saif al Harthi
  • 2,948
  • 1
  • 21
  • 26
0

Don't do this, it is stupid and confusing unless you have a finite-state automaton.

enum MyEnum
{
    A,
    B,
    C

}
private readonly Dictionary<MyEnum, Action> _handlers = new Dictionary<MyEnum, Action>
                                                        {
    {MyEnum.A,()=>Console.Out.WriteLine("Foo")},
    {MyEnum.B,()=>Console.Out.WriteLine("Bar")},
    };

public static void ActOn(MyEnum e)
{
    Action handler = null;
    if (_handlers.TryGetValue(e, out handler) && handler != null)
    {
        handler();
    }
}
Florian Doyon
  • 4,146
  • 1
  • 27
  • 37
0

Another approach would be to do some bitwise comparison, but really not worth it again.

    private void ActWithCast(MyEnum e)
    {
        const int interest = (int)MyEnum.A | (int)MyEnum.B;
        if (0 != ((int)e & interest))
        {
            Console.Out.WriteLine("Blam");
        }
    }
Florian Doyon
  • 4,146
  • 1
  • 27
  • 37
0

If the ElementType is an enum there is a shorter way to do it:

[Flags]
public enum ElementType
{
    Type1 = 1,
    Type2 = 2,
    Type3 = 4,
}
...
tElementType.HasFlag(ElementType.Type1 | ElementType.Type2);

You do not need the [Flags] attribute to use HasFlag, but the values of each of them do need to follow that pattern.

Rangoric
  • 2,739
  • 1
  • 18
  • 18