Not all code paths return a value in my controller method

Question

i am currently doing a Login page which directs to whichever page according to their StaffRole. Eg, if StaffRole = Manager, direct to manager page. Here is my code below for my controller method. However my controller method shows an error which says not all code paths return a value. I am unsure how to solve this.

` [HttpPost]
    public ActionResult Verify(Account acc)
    {
        connectionString();
        con.Open();
        com.Connection = con;
        com.CommandText = "select * from Staff where StaffNRIC='" + acc.StaffNRIC + "' and StaffContact='" + acc.StaffContact + "' and StaffAccountStatus = 'Approved'";
        dr = com.ExecuteReader();
        if (dr.Read())
        {
            if (dr.HasRows)
            {
                while (dr.Read())
                {
                    if (dr["StaffRole"].ToString() == "Manager")
                    {
                        dr.Close();
                        return RedirectToAction("Manager/ManagerHome", "ManagerController");//wherever you want to return the user to 

                    }
                    else if (dr["StaffRole"].ToString() == "Admin")
                    {
                        dr.Close();
                        return RedirectToAction("Admin/AdminHome", "AdminController");
                    }
                    else if (dr["StaffRole"].ToString() == "Part-Timer")
                    {
                        dr.Close();
                        return RedirectToAction("PartTimer/PartTimerHome", "PartTimerController");
                    }
                    else
                    {
                        con.Close();
                        return View("Login");
                    }
                }
            }
        }
    }`

' SqlConnection con = new SqlConnection(); SqlCommand com = new SqlCommand(); SqlDataReader dr;

score 1 · Answer 1 · answered Jul 22 '20 at 03:40

Just as the error message shown, not all of the path in your code returns a value. For example, if the dr.Read() is false and the code does not return anything. To solve this error, simply add return View("Login"); after the if (dr.Read()) block

[HttpPost]
public ActionResult Verify(Account acc)
{
    connectionString();
    con.Open();
    com.Connection = con;
    com.CommandText = "select * from Staff where StaffNRIC='" + acc.StaffNRIC + "' and StaffContact='" + acc.StaffContact + "' and StaffAccountStatus = 'Approved'";
    dr = com.ExecuteReader();
    if (dr.Read())
    {
        if (dr.HasRows)
        {
            while (dr.Read())
            {
                if (dr["StaffRole"].ToString() == "Manager")
                {
                    dr.Close();
                    return RedirectToAction("Manager/ManagerHome", "ManagerController");//wherever you want to return the user to 

                }
                else if (dr["StaffRole"].ToString() == "Admin")
                {
                    dr.Close();
                    return RedirectToAction("Admin/AdminHome", "AdminController");
                }
                else if (dr["StaffRole"].ToString() == "Part-Timer")
                {
                    dr.Close();
                    return RedirectToAction("PartTimer/PartTimerHome", "PartTimerController");
                }
                else
                {
                    con.Close();
                    return View("Login");
                }
            }
        }
    }
    return View("Login");
}

score 0 · Answer 2 · answered Jul 22 '20 at 04:09

like the answer by @Loong, your controller may not always return something a better way IMO would be to have an object that is set in the if/else etc. then you return that object which can be initialised to the login page. so I have added 'output' and that is what I return, setting it to something other than the login page when I can. You are also reading the same info from the data multiple times using a lookup (see my replacement with a switch)

Also maybe look at refactoring with usings so your connection is automatically disposed as below, I am assuming you are using SQLConnection and SQLCommand but this methodology should work for any iDisposable object

Also when using sql command text, maybe use SQL Variables like @accName or whatever to limit the possibility of someone calling this function and using sql injection (I haven't done that here).

[HttpPost]
public ActionResult Verify(Account acc)
{
    ActionResult output = View("Login");

    //Usings auto close and dispose of objects once you break out of them
    using(var conn = GetNewConnection()) {
        using(var cmd = new SQLCommand("select * from Staff where StaffNRIC='" + acc.StaffNRIC + "' and StaffContact='" + acc.StaffContact + "' and StaffAccountStatus = 'Approved'", conn)) {
            dr = com.ExecuteReader();

            if(dr.Read()) {
                bool isFound = false;

                if(dr.HasRows()) {
                    while(dr.read()) {
                       var role = dr["StaffRole"].ToString();

                       switch(role) {
                          case: "Manager": output = RedirectToAction("Manager/ManagerHome", "ManagerController"); isFound=true; break;
                          case: "Admin": output = RedirectToAction("Admin/AdminHome", "AdminController"); isFound=true; break;
                          case: "StaffRole": output = RedirectToAction("PartTimer/PartTimerHome", "PartTimerController"); isFound = true; break;
                       }

                       if(isFound) { //This bit breaks out of the while->read of the db if the role has been found.
                           break;
                       }
                    }
                }
            }
        }

    return output;
    }

Not all code paths return a value in my controller method

2 Answers2