0

I have a function like below but it seems more if else conditions. If there any similar effective way to handle the same. This function with my code properly working.

public bool UpdateAdjustmentRequest()
{
   if (ds1.Tables[0].Rows[0]["AppLev1Req"].ToString().Trim() == "True")
    {
        if (ds1.Tables[0].Rows[0]["AppLev2Req"].ToString().Trim() == "True")
        {
            if (ds1.Tables[0].Rows[0]["AppLev3Req"].ToString().Trim() == "True")
            {
                if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
                {
                    //1234
                    if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
                    {
                        if (UpdateApprovedRequest())
                        {                                       
                              //done 
                        }
                        else
                        {
                            return false;
                        }

                    }
                    else
                    {
                        return false;
                    }
                }
                else
                {
                    //123
                    if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved"))
                    {
                        if (UpdateApprovedRequest())
                        {
                                //done
                        }
                        else
                        {
                            return false;
                        }

                    }
                    else
                    {
                        return false;
                    }
                }
            }
            else if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
            {
                //124
                if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                         //done
                    }
                    else
                    {
                        return false;
                    }

                }
                else
                {
                    return false;
                }
            }
            else
            {
                //12
                if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                          //done
                    }
                    else
                    {
                        return false;
                    }
                }
                else
                {
                    return true;
                }
            }
        }
        else if (ds1.Tables[0].Rows[0]["AppLev3Req"].ToString().Trim() == "True")
        {
            if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
            {
                //134
                if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                          //done
                    }
                    else
                    {
                        return false;
                    }

                }
                else
                {
                    return false;
                }
            }
            else
            {
                //13
                if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                         //done
                    }
                    else
                    {
                        return false;
                    }
                }
                else
                {
                    Master.ReloadParentPage();
                    return false;
                }
            }
        }
        else if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
        {
            //14
            if ((ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
            {
              if(UpdateApprovedRequest())
                {
                     //done
                }
                else
                {
                    return false;
                }
            }
            else
            {
                return false;
            }
        }
        else
        {
            //1
            if (ds1.Tables[0].Rows[0]["Approver1Status"].ToString().Trim() == "Approved")
            {
                if (UpdateApprovedRequest())
                {
                          //done
                }
                else
                {
                  return false;
                }

            }
            else
            {
                return false;
            }
        }

    }
    else if (ds1.Tables[0].Rows[0]["AppLev2Req"].ToString().Trim() == "True")
    {
        if (ds1.Tables[0].Rows[0]["AppLev3Req"].ToString().Trim() == "True")
        {
            if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
            {
                //234
                if ((ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                         //done
                    }
                    else
                    {
                        return false;
                    }

                }
                else
                {
                    return false;
                }
            }
            else
            {
                //23
                if ((ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved"))
                {
                    if (UpdateApprovedRequest())
                    {
                       //done
                    }
                    else
                    {
                        return false;
                    }

                }
                else
                {
                    return false;
                }
            }
        }
        else if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
        {
            //24
            if ((ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
            {
                if (UpdateApprovedRequest())
                {
                     //done
                }
                else
                {
                    return false;
                }

            }
            else
            {
                return false;
            }
        }
        else
        {
            //2
            if (ds1.Tables[0].Rows[0]["Approver2Status"].ToString().Trim() == "Approved")
            {
                if (UpdateApprovedRequest())
                {
                   //done
                }
                else
                {
                    return false;
                }

            }
            else
            {                            return false;
            }
        }
    }
    else if (ds1.Tables[0].Rows[0]["AppLev3Req"].ToString().Trim() == "True")
    {
        if (ds1.Tables[0].Rows[0]["AppLev4Req"].ToString().Trim() == "True")
        {
            //34
            if ((ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved") && (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved"))
            {
                if (UpdateApprovedRequest())
                {
                       //done
                }
                else
                {
                    return false;
                }

            }
            else
            {
                return false;
            }
        }
        else
        {
            //3
            if (ds1.Tables[0].Rows[0]["Approver3Status"].ToString().Trim() == "Approved")
            {
                if (UpdateApprovedRequest())
                {
                      //done
                }
                else
                {
                    return false;
                }

            }
            else
            {
                return false;
            }
        }
    }
    else
    {
        //4
        if (ds1.Tables[0].Rows[0]["Approver4Status"].ToString().Trim() == "Approved")
        {
            if (UpdateApprovedRequest())
            {
                 //done
            }
            else
            {
                return false;
            }
        }
        else
        {
            return false;
        }
    }
    return true;
}
2
  • 1
    First suggestion create a local variable like var AppLev1Req =ds1.Tables[0].Rows[0]["AppLev1Req"].ToString().Trim() and then write if else it will be more clear. You can use and and or operator as required. The replacement of if else is switch but not sure in your case. Commented Mar 23, 2021 at 5:37
  • 1
    Why don't you just check if approver level x is required, then check if approver level x status is approved and repeat for every x between 1 and 4? You don't need a condition for every use case in the world. Also you might want to split the parsing of the DB table and this business logic to two seperate units of code (Single responsibility) Commented Mar 23, 2021 at 15:28

1 Answer 1

1

I haven't read your whole code because it is quite lengthy. But as far as I can see, you have this pattern in your code:

if(FirstCondition())
{
   if(SecondCondition())
   {
      if(ThirdCondition())
      {
          DoSomething();
          return true;
      }
      else
      {
         return false;
      }
   }
   else
   {
      return false;
   }
}
else
{
   return false;
}

Which is really difficult to read, because the if and else blocks tend to be really far away from each other (at least at the outer ifs). You can easiliy avoid it by negating the conditions:

if(!FirstCondition())
{
   return false;
}
if(!SecondCondition())
{
   return false;
}
if(!ThirdCondition())
{
   return false;
}

DoSomething();
return true;

This code is much clearer: Here, one can see immediately, that three preconditions are checked, and that some action is performed, if they are fulfilled, otherwise the method is left with a negative result.

Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.