accordo777
accordo777

Reputation: 407

Better If statement with type checking C#

I'm currently working on a .NET 4.7.1 application. I have an If-statement to check the data type and call an handler method accordingly.

My current If statement looks like this:

// object msg = MyHelper.Deserialize(xmlString);

if (msg is Tomato) Handle_Tomato((Tomato)msg);
if (msg is Apple) Handle_Apple((Apple)msg);
if (msg is Banana) Handle_Banana((Banana)msg);
if (msg is Orange) Handle_Orange((Orange)msg);

msg is basically an object deserialized from a string.

I was wondering, if there is a better way to write my if statements?

Thank you very much!

Upvotes: 4

Views: 503

Answers (4)

Andrea Rossini
Andrea Rossini

Reputation: 330

Use a dictionary. I foresee your if will explode with new cases in future, generally speaking, walls of if and large switch statements are bad code. In a similar situation I created something like this:

private static readonly Dictionary<RuntimeTypeHandle, Action<object>> handleMsgImplementations = new Dictionary<RuntimeTypeHandle, Action<object>>
{
    { typeof(Tomato).TypeHandle, x => Handle_Tomato((Tomato)x) },
    // etc...
};

// Then instead of if, use this (prepare a catch for Invalid Key or use a TryGetValue)
handleMsgImplementations[msg.GetType().TypeHandle](msg);

I get TypeHandle because I like to use a value type for the key.

EDIT: @TheGeneral answer is the best, also, the C# compiler creates a dictionary under the hood when the amount cases starts to damage performance. I keep my answer because I believe adds value.

Upvotes: 1

devsmn
devsmn

Reputation: 1040

I'd strongly suggest not to do such checks. What if in the future there are dozens of different types? Your if statement will increase and be unmaintainable. What if the type changes? You'd have to change all the if statements as well.

You could solve this by using an interface. You already have the classes.

interface IHandler
{
    void Execute();
} 
class Orange : IHandler 
{
    public void Execute()
    {
        // do your orange stuff 
    }
}
class Tomato : IHandler
{
    public void Execute()
    {
        // do your tomato stuff
    }
}

It can be called like this.

if (msg is IHandler) ((IHandler)msg).Execute();

Upvotes: 5

Magnus
Magnus

Reputation: 46947

I think the easiest would be to use a switch/case

switch (msg)
{
    case Tomato t:
        Handle_Tomato(t);
        break;
    case Apple a:
        Handle_Apple(a);
        break;
    case Banana b:
        Handle_Banana(b);
        break;
    case Orange o:
        Handle_Orange(o);
        break;
}

Upvotes: 2

TheGeneral
TheGeneral

Reputation: 81513

As Sweeper mentions in the comments, from C# 7.0 you can use the The is type pattern expression

if (msg is Tomato tomato) Handle_Tomato(tomato);

You could also use pattern matching with a switch statement (Type pattern) since C# 7.0

The type pattern enables concise type evaluation and conversion. When used with the switch statement to perform pattern matching, it tests whether an expression can be converted to a specified type and, if it can be, casts it to a variable of that type.

switch(msg)
{
   case Tomato tomato : Handle_Tomato(tomato); break;
   case Apple apple : Handle_Apple(apple); break;
   ...
}

Upvotes: 8

Related Questions