Getting Error: Not all code paths a value

Question

public bool loginpro(string loginas, string dept, string usnm, string pass)
{
    try
    {
        string qrstr;
        qrstr = "select * from login where loginas=='" + loginas + "',dept=='" + dept + "',usnm=='" + usnm + "',pass=='" + pass + "'";
        Gencon.Open();
        SqlCommand cmd = new SqlCommand(qrstr, Gencon);
        SqlDataAdapter da = new SqlDataAdapter(cmd);
        DataTable dt = new DataTable();
        da.Fill(dt);
        Gencon.Close();
        if (dt.Rows.Count > 0)
        {
            return true;
        }


    }
    catch (Exception e)
    {
        return false;
    }
}

What if Rows.Count is zero? Did you return a value? The compiler (and the C# rules) aren't happy with this. — Steve, Jun 07 '16 at 07:52
By the way, after resolving this, your query is totally wrong for many reasons. == is a C# operator, not SQL, WHERE multiple conditions should be joined by a logical operator, the sql text is an Sql Injection party. — Steve, Jun 07 '16 at 07:54

score 0 · Answer 1 · answered Jun 07 '16 at 07:53

0

problem is in try block, try block returns value only when DataTable has rows, what if you have no rows?

if (dt.Rows.Count > 0)
{
    return true;
}
else
{
    // has to return something.
    return false; 
}

or you could just simplify this using

return dt.Rows.Count > 0 ;  // assuming in else you want to return false.

answered Jun 07 '16 at 07:53

Hari Prasad

16,716
4
21
35

in else block if i want to throw the exception to catch block then? – Swaroop Jun 07 '16 at 08:02

score 0 · Accepted Answer · edited May 23 '17 at 12:16

There are many problems in your code. Of course the compiler stops you at compile time, but then you will get other errors at runtime

So fixing the compile time problem is easy. Just write a retrun value if you don't have any rows returned by your query:

    // This returns true if you have rows, false if not
    return (dt.Rows.Count > 0);

Now the problems that you will face at runtime are the following

The equal operator in SQL is = not ==
The multiple WHERE conditions should be joined by a logical operator (AND, OR)
The sql text should be parameterized

.

public bool loginpro(string loginas, string dept, string usnm, string pass)
{
    try
    {
        string qrstr;
        qrstr = @"select * from login where loginas=@login and dept = @dept
                 and usnm = @user and pass= @pass";
        Gencon.Open();
        SqlCommand cmd = new SqlCommand(qrstr, Gencon);
        cmd.Parameters.Add("@login", SqlDbType.NVarChar).Value = loginas;
        cmd.Parameters.Add("@dept", SqlDbType.NVarChar).Value = dept;
        cmd.Parameters.Add("@user", SqlDbType.NVarChar).Value = usnm;
        cmd.Parameters.Add("@pass", SqlDbType.NVarChar).Value = pass;
        SqlDataAdapter da = new SqlDataAdapter(cmd);
        DataTable dt = new DataTable();
        da.Fill(dt);
        Gencon.Close();
        return (dt.Rows.Count > 0);
    }
    catch (Exception e)
    {
        Gencon.Close();
        return false;
    }
}

There are other problems like not using the using statement and trying to pass a clear text password to your database engine that could cause memory leaks and security problems.

Dmitry Bychenko · Answer 3 · 2016-06-07T09:08:44.707

The immediate cause of the error is that .Net doesn't know what value to return if you have zero rows:

  ...
  if (dt.Rows.Count > 0)
  {
       return true;
  }
  ...
  // what should be returned? true or false?

I suggest re-writing the method into something like this:

public bool loginpro(string loginas, string dept, string usnm, string pass) {
  //DONE: Make SQL readable; debug it ( "=" instead of "==" )
  //DONE: Do not fetch redundant data (select * ...) 
  //DONE: Make SQL parametrized  
  String sql = 
    @"select 1
        from login
       where loginas = @prm_loginas and 
             dept = @prm_ dept and
             usnm = @prm_user and 
             pass = @pass"; //TODO: do not store password, but its hash value
  try { 
    //DONE: wrap IDisposable into using
    //DONE: do not use global SQL connections Gencon.Open()...Gencon.Close()
    using (SqlConnection con = new SqlConnection(connectionStringHere)) {
      con.Open();

      //DONE: wrap IDisposable into using
      using (SqlCommand cmd = new SqlCommand(sql, con)) {
        cmd.Parameters.Add("@prm_loginas", SqlDbType.NVarChar).Value = loginas;
        cmd.Parameters.Add("@prm_ dept", SqlDbType.NVarChar).Value = dept;
        cmd.Parameters.Add("@prm_user", SqlDbType.NVarChar).Value = usnm;
        //TODO: do not pass password! Pass hash value instead
        cmd.Parameters.Add("@pass", SqlDbType.NVarChar).Value = pass;

        //DONE: wrap IDisposable into using
        //DONE: do not fetch redundant data (you want at most one record only) 
        using (var reader = cmd.ExecuteReader()) {
          return reader.Read(); // <- cursor has at least one record
        }
      }  
    }
  }
  catch (DbException ee) { //DONE: do not catch all the exceptions
    return false;
  }
}

Getting Error: Not all code paths a value

3 Answers3