CODEWITHSUNDEEP
c#winformsif-statementdictionaryswitch-statement
Rodney Wilson
Rodney Wilson

Reputation: 369

C# Better way to handle multiple if/else if statements

I have a class that handles incoming over-the-air messages and parses them. Depending on the output of the command, I need to handle some UI modifications such as highlighting labels, adding text to textboxes etc. The first option I was using is: 

void IncomingMessageIfStatements(Message msg, Host host)
        {
            byte resp;
            if (ParseMessageOptionOne(msg, out resp))
            {
                // Do some windows form stuff
            }    

            else if (ParseMessageOptionTwo(msg, out resp))
            {
                // Do some windows form stuff
            }

            else if (ParseMessageOptionThree(msg, out resp))
            {
                // Do some windows form stuff
            }
        }

        private bool ParseMessageOptionOne(Message msg, out byte resp)
        {
            throw new NotImplementedException();
        }
        private bool ParseMessageOptionTwo(Message msg, out byte resp)
        {
            throw new NotImplementedException();
        }
        private bool ParseMessageOptionThree(Message msg, out byte resp)
        {
            throw new NotImplementedException();
        }

This works, but I will have more else if statements and it could get ugly. The next way I looked at doing is:

void IncomingMessageSwitchStatements(Message msg, Host host)
        {
            byte resp = 0;
            byte someByte = 0;
            bool output = false;
            switch (someByte)
            {
                case 1:
                    output = ParseMessageOptionOne(msg, out resp);
                    break;
                case 2:
                    output = ParseMessageOptionTwo(msg, out resp);
                    break;
                case 3:
                    output = ParseMessageOptionThree(msg, out resp);
                    break;
                default:
                    //handle exception here
                    break;
            }

            if (output && resp == 0x01)
            {
                UpdateUiFromHere();
            }
        }

        private void UpdateUiFromHere()
        {
            // handle UI updates here
        }

This looks much cleaner and works as intended. But then I started looking at Dictionary<byte, Func<bool>> and thought maybe that was a better approach to solving the handling of multiple conditions incoming (possibly 20).

Any suggestions on the best practice I should go for, given whats needed?

Upvotes: 2

Views: 4649

Answers (4)

Will Ray
Will Ray

Reputation: 10889

Another alternative would be to use LINQ to loop through a list of delegate methods, and short-circut as soon as a true condition is found.

private delegate bool ParseMessageWrapper(Message msg, out byte resp);

void IncomingMessageSwitchStatements(Message msg, Host host)
{
    byte resp = 0;
    bool output = false;

    var parseMethods = new List<ParseMessageWrapper>()
    {
        new ParseMessageWrapper(ParseMessageOptionOne),
        new ParseMessageWrapper(ParseMessageOptionTwo),
        new ParseMessageWrapper(ParseMessageOptionThree)
    };

    output = parseMethods.Any(func => func.Invoke(msg, out resp));

    if (output && resp == 0x01)
    {
        UpdateUiFromHere();
    }
}

Upvotes: 0

Dhananjaya Kuppu
Dhananjaya Kuppu

Reputation: 1322

You can as well change the method signature of ParseMessageOptionOne, ParseMessageOptionTwo, ParseMessageOptionThree to return a response object that encapsulates output, response and use Func built-in delegate to store methods:

public struct ParseResponse
{
    public bool output;
    public byte response;
}

class ParseOptions
{
    Func<Message, ParseResponse>[] options = null;

    public ParseOptions()
    {
        options = new Func<Message, ParseResponse>[]{
            ParseMessageOptionOne,
            ParseMessageOptionTwo,
            ParseMessageOptionThree
        };
    }

    public void IncomingMessageSwitchStatements(Message msg, Host host)
    {            
        byte someByte = 2;

        var response = options[someByte](msg);

        if (response.output && response.response == 0x01)
        {
            //UpdateUiFromHere();
        }
    }

    private ParseResponse ParseMessageOptionThree(Message msg)
    {
        return new ParseResponse { output = true };
    }

 }

Upvotes: 0

Kory Gill
Kory Gill

Reputation: 7163

I would move the parsing to inside the Message class itself.

Options option = msg.GetOption();

I would also make an enum for all your options:

public enum Options {
    One,
    Two,
    None
}

Then write your code to make use of that however you need to...

if (option == Options.One)
// or
switch(option)
// etc.

Hard to recommend more since I can't tell what UpdateUiFromHere() does since it takes no parms...but this should be some good food for thought.

Upvotes: 0

Ian
Ian

Reputation: 30813

Since what you want to do is to "Call method with the same signature based on index number", you could use delegate and list them in a Dictionary (or in a List if the index starts from 0) to make your intention clear.

public delegate bool ParseMessage(Message msg, out byte resp);

And then list it using Dictionary:

Dictionary<byte, ParseMessage> parser = new Dictionary<byte, ParseMessage>(){
    {1, new ParseMessage(ParseMessageOptionOne)},
    {2, new ParseMessage(ParseMessageOptionTwo)},
    {3, new ParseMessage(ParseMessageOptionThree)}
};

or using List

List<ParseMessage> parser = new List<ParseMessage>(){
    new ParseMessage(ParseMessageOptionOne),
    new ParseMessage(ParseMessageOptionTwo),
    new ParseMessage(ParseMessageOptionThree)
};

And call it like this:

bool result = parser[resp](msg, out resp); //dictionary
bool result = parser[resp-1](msg, out resp); //list

Upvotes: 3

Related Questions