How to eliminate "else-if" statements.

Question

My function get QueryString from some Web page as a string. I need to parce it, to check, what strategy i must use.

Now my code looks ugly (i think so):

public QueryStringParser(string QueryString)
        {
            if (string.IsNullOrEmpty(QueryString))
            {
                this._mode = Mode.First;
            }
            else if (QueryString.Contains(_FristFieldName) && !QueryString.Contains(_SecondFieldName))
            {
                this._mode = Mode.Second;
            }
            else if (!QueryString.Contains(_FristFieldName) && QueryString.Contains(_SecondFieldName))
            {
                this._mode = Mode.Third;
            }
            else
            {
                throw new ArgumentException("QueryString has wrong format");
            }
        }

There must'n't be both FieldNames in one QueryString.

How to change this code to be mo readable.

Answer 1

Use Switch case.

Answer 2

The code is readable, you could use some nesting on common conditions, to make parts clearer.

You cant use Switch because the comparison has to be to a static value.

Answer 3

I would at least remove some duplication in checking the existing fields:

public QueryStringParser(string QueryString) {
    if (string.IsNullOrEmpty(QueryString))
        this._mode = Mode.First;
    else {
        bool has_1st = QueryString.Contains(_FristFieldName);
        bool has_2nd = QueryString.Contains(_SecondFieldName);
        if      ( has_1st && !has_2nd) this._mode = Mode.Second;
        else if (!has_1st &&  has_2nd) this._mode = Mode.Third;
        else
            throw new ArgumentException("QueryString has wrong format");
    }
}

Answer 4

Comments and Indentation

---

It is already good-looking.

public QueryStringParser(string QueryString)
        {
            // Input is NULL STRING
            if (string.IsNullOrEmpty(QueryString))
            {
                this._mode = Mode.First;
            }

            // Query using FirstName
            else if (QueryString.Contains(_FristFieldName) &&
                    !QueryString.Contains(_SecondFieldName))
            {
                this._mode = Mode.Second;
            }

            //Query using SecondName
            else if (!QueryString.Contains(_FristFieldName) &&
                      QueryString.Contains(_SecondFieldName))
            {
                this._mode = Mode.Third;
            }

            //Insufficient info to Query data
            else
            {
                throw new ArgumentException("QueryString has wrong format");
            }
        }

I hope she looks Beautiful to you now...

GoodLUCK!!

Answer 5

The code is readable. You could make it shorter by removing the brackets as the statements are only one-liners:

public QueryStringParser(string QueryString)
        {
            //check if string is empty
            if (string.IsNullOrEmpty(QueryString))
                this._mode = Mode.First;
            else
            {
               bool hasFirst = QueryString.Contains(_FristFieldName);
               bool hasSecond= QueryString.Contains(_SecondFieldName);

               //check if string contains first field name but not second field name
               if (hasFirst  && !hasSecond)
                   this._mode = Mode.Second;
               //check if string contains second field name but not first field name
               else if (!hasFirst && hasSecond)
                   this._mode = Mode.Third;
               //default - error
               else
                   throw new ArgumentException("QueryString has wrong format");
            }
        }

Also - removed duplicate calls to the Contains method

I wouldn't suggest using any shorthand operators as these would make the code far less readable.

Lastly - in line comments!

Answer 6

You could put the determination of the Mode into a separate method and return the Mode value; that way you can eliminate the if-else statements

public QueryStringParser(string QueryString)
{
    this._mode = DetermineMode(QueryString);
}

private Mode DetermineMode(string QueryString)
{
    // Input is NULL STRING
    if (string.IsNullOrEmpty(QueryString))
        return Mode.First;

    bool hasFirst = QueryString.Contains(_FristFieldName);
    bool hasSecond = !QueryString.Contains(_SecondFieldName);

    // Query using FirstName
    if (hasFirst && !hasSecond)
        return Mode.Second;

    //Query using SecondName
    if (!hasFirst && hasSecond)
        return Mode.Third;

    //Insufficient info to Query data
    throw new ArgumentException("QueryString has wrong format");
}

[EDIT] Removed the double checking on the existance of field names and use variables instead.

Answer 7

Use something like ReSharper or Refactor Pro! to help tidy up your code and encourage good practice.