Könnte dieser ExecuteScalar-Aufruf besser geschrieben werden?

  • Ich bin heute in unserem Projekt auf diesen Code gestoßen. Wo immer möglich, versuche ich, die Codebasis in einer besseren Form zu verlassen, als ich sie gefunden habe, während ich weiter gehe, und diese Methode sprang aus verschiedenen Gründen auf mich zu, hauptsächlich die SQL-Zeichenfolge und den Try / Catch-Block. Ich denke, es gibt einen kostengünstigeren Weg, dies zu tun.

    Originalcode:

     public bool CheckSomething(string paramA, int paramB)
    {
        using (var conn = new SqlConnection(Connection))
        {
            conn.Open();
            string sqlCommand = "SELECT ColumnA FROM OurTable WHERE ColumnB = '" + paramA +
                                    "' AND ColumnC = " + paramB;
    
            using (var dbCommand = new SqlCommand(sqlCommand, conn))
            {
                int noOfRecords = -1;
                try
                {
                    noOfRecords = (int)dbCommand.ExecuteScalar();
                }
                catch (Exception ex)
                {
                }
                finally 
                {
                    dbCommand.Dispose();
                    if (conn.State == ConnectionState.Open)
                    {
                        conn.Close();
                    }
    
                    return noOfRecords > 0;
                }
            }
        }
    }
     

    Ich habe darüber nachgedacht, es so umzuschreiben, aber ich denke immer noch, dass es noch verbessert werden könnte. Eine davon wäre, eine Prozedur für die SQL zu erstellen, aber das ist unwahrscheinlich. Ziel war es, es rein aus Sicht des Codes zu verbessern. Ich würde mich über Gedanken freuen.

    Umgeschriebene Version:

     public bool CheckSomething(string paramA, int paramB)
    {
        using (var conn = new SqlConnection(Connection))
        {
            conn.Open();
            string sqlCommand = string.Format("SELECT ColumnA FROM OurTable WHERE ColumnB = '{0}' and ColumnB = {1}", paramA, paramB);
    
            using (var dbCommand = new SqlCommand(sqlCommand, conn))
            {
                object noOfRecords = dbCommand.ExecuteScalar();
                dbCommand.Dispose();
    
                if (conn.State == ConnectionState.Open)
                {
                    conn.Close();
                }
    
                return noOfRecords != null;
            }
        }
    }
     
    22 November 2011
1 answer
  •  public bool CheckSomething(string paramA, int paramB)
    {    
        using (var conn = new SqlConnection(".."))
        using (var comm = new SqlCommand("", conn))
        {
            conn.Open();
            object noOfRecords = comm.ExecuteScalar();
            return noOfRecords != null;
        }
    }
     

    Es ist nicht erforderlich, das Element zu schließen oder zu entsorgen. Das Element using behandelt diesen Teil. Dadurch entfällt die Notwendigkeit einer manuellen Try-Catch- oder Schließ-Logik, wodurch ein viel komprimierter Codeabschnitt übrig bleibt, der funktional gleichwertig und ebenso sicher ist Verwenden Sie parametrisierte SQL oder eine gespeicherte Prozedur im Gegensatz zur Verkettung von Zeichenfolgen. Parametrisierte SQL:

     string sql = "SELECT ColumnA FROM OurTable WHERE ColumnB = @param1 AND ColumnC = @param2";
    
    using (var comm = new SqlCommand(sql, conn))
    {
        comm.Parameters.AddWithValue("@param1", param1);
        comm.Parameters.AddWithValue("@param2", param2);
    
        conn.Open();
        // etc...
    }
     
    22 November 2011
    Adam Houldsworth