I made such a construction, to add data to the database, the question, was the query written correctly, and if it is taken into account that this is a cycle, would such a solution be effective?

using (SqlConnection connection =new SqlConnection(connextionstring)) { SqlCommand command =new SqlCommand("DELETE FROM Class1", connection); connection.Open(); command.ExecuteNonQuery(); SqlCommand command2 = new SqlCommand("DELETE FROM Class2", connection); command2.ExecuteNonQuery(); SqlCommand command3 = new SqlCommand("DELETE FROM Class3", connection); command3.ExecuteNonQuery(); SqlCommand command4 = new SqlCommand("DELETE FROM Class4", connection); command4.ExecuteNonQuery(); SqlCommand command5 = new SqlCommand("DELETE FROM Class5", connection); command5.ExecuteNonQuery(); Class1 contactMary = ObjectSpace.FindObject<Class1>(); foreach (var VARIABLE in mass2) { String[] words = VARIABLE.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries); var a= contactMary.METHODID = Convert.ToInt16(words[0]); var b= contactMary.DESCRIPTION = words[1]; var c= contactMary.METHODTYPEID = words[2]; var d= contactMary.TEXTDESCRIPTION = words[3]; SqlCommand command6 = new SqlCommand("INSERT INTO Class1 (METHODID, DESCRIPTION, METHODTYPEID, TEXTDESCRIPTION) VALUES (@a, @b, @c, @d)", connection); command6.Parameters.AddWithValue("@a",a); command6.Parameters.AddWithValue("@b",b); command6.Parameters.AddWithValue("@c",c); command6.Parameters.AddWithValue("@d",d); command6.ExecuteNonQuery(); }
  • Probably because you are writing invalid C # code, the compiler does not tell you anything? - tym32167
  • 2
    Parameterization queries ? - tym32167
  • 2
    That is, do you think that the good C # will take and guess that in the fragment ... SET METHODID="a", DESCRI... instead of a should substitute the value of the variable, and even correctly frame it with quotes? And I’ll guess that in the fragment ... INTO Class1 SET ... you don’t need to do this ... I’m sorry - you’ll have to do it yourself. - Akina
  • @ tym32167 Thanks for the answer, but it shows the invalidity. - Vladimr Vladimirovoch
  • one
    How is it going? If it does not work out - ask (you can edit this question). If it worked out - show your code. Why am I wondering: in the shown code there is no release of resources; and creating a SqlCommand would have to be taken out of the loop. In general, this is what pulls back the refactor ... - Alexander Petrov

1 answer 1

First, all variables must be given talking names. command2 , command3 - looking at their names, what can you guess about them? What are these commands doing? Unclear...

You can call, for example, the following: deleteClass2Command , deleteClass3Command — it’s immediately clear what they are for.

The names of the tables in the database, by the way, would also be good to rename. Because Class1 little.

To release (disposit) you need not only the connection, but all the commands. That is, they also need to be wrapped in using .

 using (var deleteClass2Command = new SqlCommand("DELETE FROM Class1", connection)) { .... }

Go ahead. In principle, it is not at all necessary to use several commands. It is enough to override a single request.

 connection.Open(); using (var deleteCommand = new SqlCommand("DELETE FROM Class1", connection)) { deleteCommand.ExecuteNonQuery(); deleteCommand.CommandText = "DELETE FROM Class2"; deleteCommand.ExecuteNonQuery(); deleteCommand.CommandText = "DELETE FROM Class3"; deleteCommand.ExecuteNonQuery(); // и т. д. }

Here the command name is common to all, so it does not specify which table it belongs to.

Further. The name of the variable contactMary is good. And the register is correctly typed, in accordance with the guides . True, it is completely unclear why it is needed.

But mass2 is bad. This is probably an array (in English array ). Array of what ? What it contains, what objects?

VARIABLE - very bad. The word variable means variable and only. And it should not be in capital letters.

Using the AddWithValue method can lead to performance problems. I advise you to read: Can we stop using AddWithValue () already? . It is better to explicitly specify data types in columns. Specify the values ​​of the SqlDbType enumeration that are in the database table.

 using (var insertCommand = new SqlCommand( "INSERT INTO Class1 (METHODID, DESCRIPTION, METHODTYPEID, TEXTDESCRIPTION) " + "VALUES (@id, @desc, @typeId, @textDesc)", connection)) { foreach (var item in someCollection) { String[] words = item.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries); var id = Convert.ToInt16(words[0]); var desc = words[1]; var typeId = words[2]; var textDesc = words[3]; insertCommand.Parameters.Add("@id", SqlDbType.SmallInt).Value = id; insertCommand.Parameters.Add("@desc", SqlDbType.NVarChar).Value = desc; insertCommand.Parameters.Add("@typeId", SqlDbType.NVarChar).Value = typeId; insertCommand.Parameters.Add("@textDesc", SqlDbType.NVarChar).Value = textDesc; insertCommand.ExecuteNonQuery(); } }

I took the team out of the cycle: there’s no need to recreate it at each iteration.
Again, changed the names of the parameters. Here you can accuse me of being too short, little-spoken ... Well, so, we are all much rant, and it’s reluctant to stuff a long code ... :)

And finally, you need to add exception handling and check return values ​​of the ExecuteNonQuery method.

  • Alexander, thanks for the detailed answer, he really helped me, at the expense of names and other things, I read clean code, and I catch myself thinking that in my case it is not at all clean, but I’m a hostage of the method, like make it clumsy first to work, then improve, remove the extra modernize. Maybe I'm wrong. - Vladimr Vladimirovoch