CODEWITHSUNDEEP
c#
Dan272
Dan272

Reputation: 213

Returning a value from a function - error

I am getting an error, I cannot return a value from the function below. Help will be appreciated.

private void UserExiest(string username)
{
    SqlConnection myConnection = new SqlConnection("user id=test;" +
            "password=test;" +
            "server=.;" +
            "Trusted_Connection=yes;" +
            "database=DB; " +
            "MultipleActiveResultSets=True;" +
            "connection timeout=30");

    myConnection.Open();
    SqlCommand CHECKNPC = new SqlCommand("select struserid from USERDATA where strUserId = '" + username + "'", myConnection);
    SqlDataReader NpcReader = CHECKNPC.ExecuteReader();
    if (NpcReader.HasRows)
    {
        return "1";
    }
    else
    {
        return "0";
    }
    myConnection.Close();
}

Upvotes: 2

Views: 113

Answers (6)

jwaliszko
jwaliszko

Reputation: 17064

private bool UserExist(string username)
{
    using (var con = new SqlConnection("..."))
    {
        con.Open();
        using (var cmd = new SqlCommand("...", con))
        {
            using (var r = cmd.ExecuteReader())
            {
                return r.HasRows;
            }
        }
    }
}

Despite the fact your orginal function doesn't return any value and doesn't close the connection - you're trying to implement unnecessary logic (returning string flags instead of returning ready to use bool value, which is the result of reader.HasRows expression). Finally remember about using statements while working with types implementing IDisposable, which guarantees to perform application-defined tasks associated with freeing, releasing, or resetting unmanaged resources (see msdn).

Upvotes: 3

Kami
Kami

Reputation: 19407

As suggested in many response here, you need to add a return type to your function. Change the void to string to return the value. As the value is a 1 or 0 it might be better to return true or false.

Also, you have your connection close statement after the return. This statement would not be triggered in normal flow of the application. Try the following :

private bool UserExiest(string username)
{
    SqlConnection myConnection = new SqlConnection("user id=test;" +
            "password=test;" +
            "server=.;" +
            "Trusted_Connection=yes;" +
            "database=DB; " +
            "MultipleActiveResultSets=True;" +
            "connection timeout=30");
    try
    {
        myConnection.Open();
        SqlCommand CHECKNPC = new SqlCommand("select struserid from USERDATA where strUserId = '" + username + "'", myConnection);
        SqlDataReader NpcReader = CHECKNPC.ExecuteReader();
        return NpcReader.HasRows;
    }
    finally
    {
        if (myConnection.State != System.Data.ConnectionState.Closed)
        {
            myConnection.Close();
        }

    }
}

I have simply added a finally block to ensure the connection is closed and changed the return types to boolean true and false.

Upvotes: 0

Habib
Habib

Reputation: 223247

Your function has return type void. You can't return a string from there. Change your function signature to :

private string UserExist(string username)

Its better if you return bool, since you are checking if NpcReader.HasRows and then returning "1" in case of true and "0" in case of false. Also its better if you close the connection, before returning the value.

Always use SqlParameter or Parameterized query , your current query is open for SQL Injection.

Upvotes: 4

devdigital
devdigital

Reputation: 34349

You method signature doesn't have a return type:

private void UserExist(string username)

You probably want to return a boolean

private bool UserExist(string username)

return true;
// or
return false;

Upvotes: 3

provençal le breton
provençal le breton

Reputation: 1438

Are you using return statements in your method, while its definition show you must return void? Change your definition methode to :

private string UserExiest(string username)

Upvotes: 0

Kobunite
Kobunite

Reputation: 365

private void UserExiest(string username)

Change this to:

private string UserExiest(string username)

seeing as you appear to be returning a string...

Upvotes: 1

Related Questions