C# not all code paths return a value

Question

I'm trying to check if a username with the given passwort exists in my database, if it exists it should return true otherwise it should return false.

My current function looks like the following:

public bool user_check(string username, string password)
{
    string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

    if (this.OpenConnection() == true)
    {
        MySqlCommand cmd = new MySqlCommand(query, connection);
        MySqlDataReader dataReader = cmd.ExecuteReader();
        if (dataReader.HasRows)
        {
            while (dataReader.Read())
            {
                return true;
            }
        }
        else
        {
            return false;
        }
        dataReader.Close();
        this.CloseConnection();
    }
}

But I receive the following error message:

Error CS0161 'database_connector.user_check(string, string)': not all code paths return a value

What am I doing wrong?

What gets returned if `OpenConnection` doesn't return `true`, for example? — Charles Mager, Aug 02 '16 at 07:34
When returning, anything below it will get ignored. so you'll also get a error saying code can't be reached at the last two lines — spatbord, Aug 02 '16 at 07:37

Gilad Green · Accepted Answer · 2016-08-02T07:42:02.243

Just add a return false; at the end. Also you don't need the else return false in the inner if

public bool user_check(string username, string password)
{
    string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

    if (this.OpenConnection())
    {
        MySqlCommand cmd = new MySqlCommand(query, connection);
        MySqlDataReader dataReader = cmd.ExecuteReader();
        if (dataReader.HasRows)
        {
            while (dataReader.Read())
            {
                return true;
            }
        }
        dataReader.Close();
        this.CloseConnection();
    }

    return false;
}

Also see that if you have records then you return true and never close your connection and object. Maybe do this instead:

public bool user_check(string username, string password)
{
    string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

    bool hasRecords = false;

    if (this.OpenConnection())
    {
        MySqlCommand cmd = new MySqlCommand(query, connection);
        MySqlDataReader dataReader = cmd.ExecuteReader();
        if (dataReader.HasRows)
        {
            while (dataReader.Read())
            {
                hasRecords = true;
                break;
            }
        }
        dataReader.Close();
        this.CloseConnection();
    }
    return hasRecords;
}

last thing: look into Parameterized Queires to avoid SQL Injections

score 2 · Answer 2 · answered Aug 02 '16 at 07:34

Just add a return false at the end of the method:

public bool user_check(string username, string password)
{
    string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

    if (this.OpenConnection() == true)
    {
        MySqlCommand cmd = new MySqlCommand(query, connection);
        MySqlDataReader dataReader = cmd.ExecuteReader();
        if (dataReader.HasRows)
        {
            while (dataReader.Read())
            {
                return true;
            }
        }
        else
        {
            return false;
        }
        dataReader.Close();
        this.CloseConnection();
    }
    return false;  //<<---- This is where it does not know what to do if any above conditions fail.
}

score 1 · Answer 3 · answered Aug 02 '16 at 07:34

1

if this check returns false:

 if (this.OpenConnection() == true)

You exit without returning anything.

answered Aug 02 '16 at 07:34

Mitch Wheat

295,962
43
465
541

score 1 · Answer 4 · edited May 23 '17 at 11:58

Error CS0161 occurs when a function that specifies a return type in its signature contains a path through the function that does not return a value. In your case your function does not return a value when the this.OpenConnection() method returns false.

To prevent this error from being reported by the compiler, all paths should return a value:

public bool user_check(string username, string password)
{
    string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

    if (this.OpenConnection() == true)
    {
        MySqlCommand cmd = new MySqlCommand(query, connection);
        MySqlDataReader dataReader = cmd.ExecuteReader();
        if (dataReader.HasRows)
        {
            while (dataReader.Read())
            {
                return true;
            }
        }
        else
        {
            return false;
        }
        dataReader.Close();
        this.CloseConnection();
    }
    return false;
}

I take this opportunity to let you know about SQL injection

Your code is vulnerable to SQL injection because you're concatenating user input into your query. You're encouraged to use parameterized queries, for more information about this topic check out this link

your code will still get the exception.. you forgot the case where it won't enter the `OpenConnection == true` if — Gilad Green, Aug 02 '16 at 07:45
@GiladGreen It was my intention to place the return false there as I typed in the body of my answer, but forgot to actually add it to the code. Thank you for the edit :) — Wazner, Aug 02 '16 at 07:48

score 0 · Answer 5 · answered Aug 02 '16 at 07:47

That's because you are writing your return inside if while blocks only.Besides I suggest you use parameterized query

public bool user_check(string username, string password){
    string query = "SELECT username, password From swear_tool Where "+
                   "username=@uname and password=@password";
    if (this.OpenConnection() == true){
        using(MySqlCommand cmd = new MySqlCommand(query, connection)){
            cmd.Parameters.AddWithValue("@uname",usename);
            cmd.Parameters.AddWithValue("@password",password);
            using(MySqlDataReader dataReader = cmd.ExecuteReader()){
                if (dataReader.HasRows){
                    while(dataReader.Read()){
                        return true;
                    }
                }
            }
        }
        this.CloseConnection();                    
    }
    return false;
}

Nice and Clean and Safe and Shorter

score 0 · Answer 6 · answered Aug 02 '16 at 08:00

The previous answer do not close the connection when a user exists. Try this:

    public bool user_check(string username, string password)
    {
        string query = "SELECT username, password from swear_tool where username='" + username + "' and password = '" + password + "'";

        if (this.OpenConnection())
        {
            try
            {
                using (MySqlCommand cmd = new MySqlCommand(query, connection))
                {
                    using (MySqlDataReader dataReader = cmd.ExecuteReader())
                    {
                        if (dataReader.HasRows)
                        {
                            while (dataReader.Read())
                            {
                                return true;
                            }
                        }
                    }
                }

            }
            finally
            {
                this.CloseConnection();
            }
        }
        return false;
    }

C# not all code paths return a value

6 Answers6