How is wrong on function ExecuteNonQuery to work as Best Pra

I work on c# app I need to make function make insert data or update or delete dynamically so that I do function below for insert

or update or delete but I don't know what must added or remove from function below to make function work as best practice .

public static async Task<int> ExecuteNonQuery(string sql, SqlConnection sqlconnection, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{
int RecordsCount = 0;
if (sql == "") return 0;
await Task.Run(async () =>
{
using (var con = new SqlConnection(GlobalVariables.con))
{
using (var cmd = new SqlCommand() { Connection = con })
{
if (cmd.CommandTimeout < 360)
cmd.CommandTimeout = 360;
cmd.CommandText = sql;
cmd.CommandType = cmdType;
cmd.Parameters.Clear();
if (@params != null)
{
for (int i = 0; i < @params.Length; i++)
{
cmd.Parameters.Add(@params[i]);
}
}
try
{
await con.OpenAsync();
RecordsCount = (await cmd.ExecuteNonQueryAsync());
}
catch (Exception ex)
{
throw new Exception(ex.Message);
}
}
}
});
return RecordsCount;
}

so I do function above for make insert or update or delete

what is remaining or wrong to be best practice ?

There is nothing in the world which you can say perfect (especially in IT industry, where in every step there is an improvement) but you can consider the following points to make a little more modular.

Avoid using static functions unless necessary because it holds the memory until your application process ends.
You had used new SqlConnection at line 9 where as you had already declared in its parameter (which seems useless for working)
Create multiple variant of your function by giving more option to developer as below

public async Task<int> ExecuteNonQuery(string sql, SqlConnection sqlconnection, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{
}
public async Task<int> ExecuteNonQuery(string sqlQuery)
{
}
public async Task<int> ExecuteNonQuery(string sqlQuery, SqlConnection sqlconnection=null, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{
}
public async Task<int> ExecuteNonQuery(SqlCommand sqlCmd, SqlConnection sqlconnection=null, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{
}

and so on...

This will give developer a more independent choice and pass there parameter accordingly

Good Luck

1 Reply

Know the answer? Post it — somebody with the same question will find it here.

Amit Gupta6y ago
There is nothing in the world which you can say perfect (especially in IT industry, where in every step there is an improvement) but you can consider the following points to make a little more modular.
Avoid using static functions unless necessary because it holds the memory until your application process ends.
You had used new SqlConnection at line 9 where as you had already declared in its parameter (which seems useless for working)
Create multiple variant of your function by giving more option to developer as below
public async Task<int> ExecuteNonQuery(string sql, SqlConnection sqlconnection, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{

}

public async Task<int> ExecuteNonQuery(string sqlQuery)
{

}

public async Task<int> ExecuteNonQuery(string sqlQuery, SqlConnection sqlconnection=null, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{

}

public async Task<int> ExecuteNonQuery(SqlCommand sqlCmd, SqlConnection sqlconnection=null, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{

}
and so on...

This will give developer a more independent choice and pass there parameter accordingly

Good Luck