CODEWITHSUNDEEP
asp.netsqlsql-server
user3292642
user3292642

Reputation: 761

multiple SQL commands. Good style?

I got an asp.net application running perfectly fine. in my code i have the following lines

using (SqlConnection con = new SqlConnection(CS))
{
    SqlCommand getGenreId = new SqlCommand("Select ID from tblGenre WHERE Genre=@newGenre;", con);
    getGenreId.Parameters.AddWithValue(@"newGenre", newGenre);
    SqlCommand cmd = new SqlCommand("UPDATE tblSong SET Title=@newTitle, ArtistId=@newArtistId, GenreId=@newGenreId WHERE (ID = @songId);", con);
    cmd.Parameters.AddWithValue(@"newTitle", newTitle);
    cmd.Parameters.AddWithValue(@"newArtistId", newArtistId);
    cmd.Parameters.AddWithValue(@"songId", songId);
    con.Open();
    newGenreId = (int)getGenreId.ExecuteScalar();
    cmd.Parameters.AddWithValue(@"newGenreId", newGenreId);
    cmd.ExecuteNonQuery();
}

i know database connections are valuable resources and i should be careful when using them. (open as late as possible and make sure they will be closed aswell)

My question now is this code considered bad style because im opening the connection then have as sql query to get an ID and then have another sql query to insert a record.

thanks you!

Upvotes: 0

Views: 45

Answers (3)

Bacon Bits
Bacon Bits

Reputation: 32170

Why not use a single query with a subquery for your SQL?

UPDATE tblSong SET Title = @newTitle, ArtistId = @newArtistId, GenreId = (Select top 1 ID from tblGenre WHERE Genre=@newGenre ORDER BY Genre) WHERE (ID = @songId);

Upvotes: 0

user275683
user275683

Reputation:

If you convert to using stored procedure, you can eliminate 1 round trip, therefore reducing network traffic and possibly increase performance.

using (SqlCommand cmd = new SqlCommand("Update_tblSong", con);
{
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.AddWithValue("@newGenre", newGenre);
    cmd.Parameters.AddWithValue("@newTitle", newTitle);
    cmd.Parameters.AddWithValue("@newArtistId", newArtistId);
    cmd.Parameters.AddWithValue("@songId", songId);
    cmd.ExecuteNonQuery();
}

Proc will be like this, I estimated on your variable size.

  CREATE PROC Update_tblSong
    (
     @newGenre VARCHAR(25)
    ,@newTitle VARCHAR(50)
    ,@newArtistID INT
    ,@songID INT
    )
  AS
    BEGIN 
        SET NOCOUNT ON;
        DECLARE @NewGenreID INT;

        SELECT @NewGenreID = ID
            FROM tblGenre
            WHERE Genre = @newGenre; 

        UPDATE tblSong
            SET Title = @newTitle
               ,ArtistId = @newArtistId
               ,GenreId = @NewGenreID
            WHERE ( ID = @songId )
    END;

Upvotes: 3

Habib
Habib

Reputation: 223267

Overall your code flow seems fine, you are using a single connection to execute multiple (related) commands.

You can improve it further with enclosing your command objects in using statement. Since they implement IDisposable interface, just like your connection object. 

using (SqlConnection con = new SqlConnection(CS))
{
    con.Open();
    using (SqlCommand getGenreId = new SqlCommand("Select ID from tblGenre WHERE Genre=@newGenre;", con))
    {
        getGenreId.Parameters.AddWithValue(@"newGenre", newGenre);
        newGenreId = (int)getGenreId.ExecuteScalar();
    }
    using (SqlCommand cmd = new SqlCommand("UPDATE tblSong SET Title=@newTitle, ArtistId=@newArtistId, GenreId=@newGenreId WHERE (ID = @songId);", con))
    {
        cmd.Parameters.AddWithValue(@"newTitle", newTitle);
        cmd.Parameters.AddWithValue(@"newArtistId", newArtistId);
        cmd.Parameters.AddWithValue(@"songId", songId);
        cmd.Parameters.AddWithValue(@"newGenreId", newGenreId);
        cmd.ExecuteNonQuery();
    }
}

Upvotes: 2

Related Questions