How is wrong on function ExecuteNonQuery to work as Best Practise ?

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 .

Code:

---

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 ?

I've already answered this question on another forum almost a full day ago. I specified six changes that I thought you should make. You asked me to write the changed code for you and I asked what was stopping from following the suggestions provided and writing it for yourself. I heard no further reply but now I find that you have simply copied and pasted your first post from that forum to here. You've ignored everything I said and made no effort to do anything for yourself and are now asking others to provide information that you already have. Did you even read what I posted there? Why so lazy?

Just so everyone else knows what you've already been provided with and chosen to ignore, here's what I provided elsewhere:

Quote:

---

• You're passing in a SqlConnection but ignoring it and creating a new one.
• You have two nested using blocks with no code between them so you only need one, i.e. rather than this:
  
  csharp Code:

  using (var x = new SomeType())
  {
      using (var y = new SomeOtherType())
      {
          // ...
      }
  }

  use this:
  
  csharp Code:

  using (var x = new SomeType())
  using (var y = new SomeOtherType())
  {
      // ...
  }

• You are making poor use of an object initialiser here:
  
  csharp Code:

  using (var cmd = new SqlCommand() { Connection = con })
  {
      if (cmd.CommandTimeout < 360)
          cmd.CommandTimeout = 360;
   
      cmd.CommandText = sql;
      cmd.CommandType = cmdType;

  You use a parameterless constructor and then set two properties that could be set via a different constructor and you set one property via an object initialiser and then set three more conventionally. That if statement is also pointless, given that you just created the command object and know exactly what the CommandTimeout is. That code should be like this:
  
  csharp Code:

  using (var cmd = new SqlCommand(sql, con) { CommandType = cmdType, CommandTimeout = 360 })
  {

• What's the point in clearing the Parameters collection of a command object that you just created and thus know has no parameters?
• The Parameters collection has an AddRange method so there's no need for a loop.
• What you're doing with that try...catch is bad a number of reasons. Why are you creating your own exception with the same message instead of allowing all the information contained in the original exception to bubble up? If you have a valid reason for creating your own exception then you should be providing your own message and setting the original exception as its InnerException. If you're not going to create your own exception but you have something else useful to do, e.g. log the exception, then you should just rethrow the original exception. You can do that using throw ex; but that's bad because it truncates the stack trace to the current method. To rethrow the original exception in its full glory, just use throw;. That said, if you're not doing anything in your catch block but rethrowing an exception then you should not be catching it at all, because the end result is the same and you don't waste the time of catching an rethrowing.

---