CODEWITHSUNDEEP
c#.net
Natifan
Natifan

Reputation: 11

How to make this if statement with multiple conditions shorter?

I began fixing a problem the customer had with signatures not displaying correctly. The customer has 2 columns one for current and for prior times - so they can have five outcomes:

Current new  These two we just need to show the CurrDate
Prior New      This hits the 1st ret

Current New    This hits the 3rd ret
Prior Null

Current old  These three we need to show the PrevDate
Prior old      This hits the 2nd ret

Current new    This hits the 2nd ret
Prior old

Current old    This hits the 4th ret
Prior null

I and got to this if statement

    private string CheckBoxDescription()
    {
        if (ViewState["PrevDate"] != null &&
            ViewState["PrevDate"].ToString().Trim() != "" && 
            ViewState["CurrDate"] != null && 
            ViewState["CurrDate"].ToString().Trim() != "")
        {
            if (DateTime.Parse(ViewState["PrevDate"].ToString()) > 
                   DateTime.Parse("01-JAN-12") &&
                DateTime.Parse(ViewState["CurrDate"].ToString()) >
                  DateTime.Parse("01-JAN-12"))
            {
                return "SELECT Statement to get CurrDate;
            }
            else
            {
                return "SELECT Statement to get PrevDate;
            }
        }

        if (DateTime.Parse(ViewState["CurrDate"].ToString()) >
            DateTime.Parse("01-JAN-12"))
        {
            return "SELECT Statement to get CurrDate (Same as the CurrDate above);
        }
        else
        {
            return "SELECT Statement to get PrevDate (Same as the PrevDate above);
        }
    }

I tried this: 

    if (DateTime.Parse(ViewState["CurrDate"].ToString()) > 
          (DateTime.Parse("011-JAN-12")) &
       (DateTime.Parse(ViewState["PrevDate"].ToString()) > 
          (DateTime.Parse("011-JAN-12")) | 
       (DateTime.Parse(ViewState["PrevDate"].ToString()) !=
           null)))

but it will not pick up one of the null values.

Upvotes: 0

Views: 302

Answers (5)

user1788956
user1788956

Reputation:

How about this;

private string CheckBoxDescription()
{
    var prevDate = ViewState["PrevDate"] != null && ViewState["PrevDate"].ToString().Trim() != ""
    ? DateTime.Parse(ViewState["PrevDate"].ToString()) 
    : null;
    var currDate = ViewState["PrevDate"] != null && ViewState["CurrDate"].ToString().Trim() != "" 
    ? DateTime.Parse(ViewState["CurrDate"].ToString()) 
    : null;

    if (prevDate != null && currDate != null)
    {

       return 
        prevDate > DateTime.Parse("01-JAN-12") && currDate > DateTime.Parse("01-JAN-12") 
        ? "SELECT Statement to get CurrDate" 
        : "SELECT Statement to get PrevDate";
    }

    if (currDate > DateTime.Parse("01-JAN-12"))
    {
        return "SELECT Statement to get CurrDate (Same as the CurrDate above);
    }
    else
    {
        return "SELECT Statement to get PrevDate (Same as the PrevDate above);
    }
}

Upvotes: 0

Matthew Watson
Matthew Watson

Reputation: 109547

You can simplify this considerably by writing a method to parse a string into a nullable date:

private static DateTime? parseDate(object date)
{
    DateTime result;

    if (DateTime.TryParse(date.ToString(), out result))
        return result;

    return null;
}

Note that I had to make the parameter an object because I don't know what type ViewState["key"] returns.

One you have that you can simplify your code. However, doing so reveals that you haven't handled the case where currDate is not a valid date:

private string CheckBoxDescription()
{
    var prevDate = parseDate(ViewState["PrevDate"]);
    var currDate = parseDate(ViewState["CurrDate"]);

    if (prevDate != null && currDate != null)
    {
        return 
            prevDate > DateTime.Parse("01-JAN-12") && currDate > DateTime.Parse("01-JAN-12") 
            ? "SELECT Statement to get CurrDate" 
            : "SELECT Statement to get PrevDate";
    }

    // You have a problem here. What if currDate is not a valid date?

    if (currDate == null)
        throw new InvalidOperationException("Your current code doesn't handle this case.");

    if (currDate > DateTime.Parse("01-JAN-12"))
    {
        return "SELECT Statement to get CurrDate (Same as the CurrDate above)";
    }
    else
    {
        return "SELECT Statement to get PrevDate (Same as the PrevDate above)";
    }
}

Upvotes: 0

D Stanley
D Stanley

Reputation: 152501

For the null/empty check you can use String.IsNullOrWhitespace:

if (!string.IsNullOrWhiteSpace(ViewState["PrevDate"]) &&
    !string.IsNullOrWhiteSpace(ViewState["CurrDate"]))

I would also store the parsed dates in variables rather than parsing each time

DateTime minDate  = new DateTime(2012,1,1);
DateTime prevDate = DateTime.Parse(ViewState["PrevDate"]));
DateTime currDate = DateTime.Parse(ViewState["CurrDate"]));

then use those in the comparisons:

if (prevDate > minDate &&
    currDate > minDate)

For the last check, DateTime.Parse will never return null so I'm not sure what conditions you're trying to check there. If the string value could be null then just check that:

if (currDate > minDate &
       (ViewState["PrevDate"] == null ||
        prevDate > minDate)
   )

Upvotes: 3

Nonagon
Nonagon

Reputation: 407

Your friend: Case statements https://msdn.microsoft.com/en-GB/library/06tc147t.aspx

Multiple if statements also have a habit of slowing your code down because the IDE has to process all the conditions

With Case statements, it will only use certain pieces of information based on conditions

Upvotes: 0

Brendon
Brendon

Reputation: 334

 private string CheckBoxDescription()
{
    if (ViewState["PrevDate"] != null &&
        ViewState["PrevDate"].ToString().Trim() != "" && 
        ViewState["CurrDate"] != null && 
        ViewState["CurrDate"].ToString().Trim() != "")
    {
        if (DateTime.Parse(ViewState["PrevDate"].ToString()) > 
               DateTime.Parse("01-JAN-12") &&
            DateTime.Parse(ViewState["CurrDate"].ToString()) >
              DateTime.Parse("01-JAN-12"))
        {
            return "SELECT Statement to get CurrDate;
        }
            return "SELECT Statement to get PrevDate;
    }

    if (DateTime.Parse(ViewState["CurrDate"].ToString()) >
        DateTime.Parse("01-JAN-12"))
    {
        return "SELECT Statement to get CurrDate (Same as the CurrDate above);
    }
        return "SELECT Statement to get PrevDate (Same as the PrevDate above);
}

From what you have done in the second block of code this is was what i could change, not much but what i have done is removed the else statements as you are returning i the first if statement, the else isn't needed.

Upvotes: 0

Related Questions