Save to a MySQL database a temporary row with all information about a receipt

Question

The Code
The Save Function:

public void SaveProducts()
{ 
 TempSave _Save = new TempSave();
 _Save.Localization = _LocationLabel.Text + "," + _LocNumbLab.Text;
 string items = "";
 for (int i = 0; i < _ProductsGrid.Rows.Count; i++)
 {
 items += _ProductsGrid.Rows[i].Cells[1].Value.ToString();
 items += ",";
 } 
 string Qnt = "";
 for (int i = 0; i < _ProductsGrid.Rows.Count; i++)
 {
 Qnt += _ProductsGrid.Rows[i].Cells[0].Value.ToString();
 Qnt += ",";
 }
 try
 {
 items = items.Remove(items.Length - 1, 1);
 Qnt = Qnt.Remove(Qnt.Length - 1, 1);
 }
 catch{}
 _Save.Items = items;
 _Save.Quantity = Qnt;
 IEnumerable<TempSave> _CheackTem;
 string Cheak_TempSave = "SELECT * FROM TempSave WHERE Localization='" + _Save.Localization + "'";
 using (MySqlConnection Con = new MySqlConnection("server=localhost; database=wintestbeta;user=root; password=;"))
 {
 Con.Open();
 _CheackTem = Con.Query<TempSave>(Cheak_TempSave);
 Con.Close();
 }
 if (_CheackTem.Count() == 0)
 {
 string _TempSave = "INSERT INTO TempSave (Localization,items,Quantity) VALUES('" + _Save.Localization + "','" + _Save.Items + "','" + _Save.Quantity + "')";
 using (MySqlConnection Con = new MySqlConnection("server=localhost; database=wintestbeta;user=root; password=;"))
 {
 Con.Open();
 Con.Query(_TempSave);
 Con.Close();
 }
 }
 else
 {
 string _TempDelete = "DELETE FROM TempSave WHERE Localization='" + _Save.Localization + "'";
 using (MySqlConnection Con = new MySqlConnection("server=localhost; database=wintestbeta;user=root; password=;"))
 {
 Con.Open();
 Con.Query(_TempDelete);
 Con.Close();
 }
 string _TempSave = "INSERT INTO TempSave (Localization,items,Quantity) VALUES('" + _Save.Localization + "','" + _Save.Items + "','" + _Save.Quantity+ "')";
 using (MySqlConnection Con = new MySqlConnection("server=localhost; database=wintestbeta;user=root; password=;"))
 {
 Con.Open();
 Con.Query(_TempSave);
 Con.Close();
 }
 }
}

The TempSave class

public class TempSave
{
 public int id { get; set; }
 public string Localization { get; set; }
 public string Items { get; set; }
 public string Quantity { get; set; }
}

I don't use the int id but I left it there because I can became necessary in the future

How It Works
What the code does is gather 3 information's from a grid

1. Get the location (this is from a label)

2. Get all items from a grid that have been selected and put it into a string array

3. Get the quantity of all items from a grid and put it into a string array

The grid looks like this

+---------------------+
|Qnt|Description|Value|
+---------------------+

Then for each string in the items string array it will join all string together with a "," between them.

Then it does the same to the quantity string array

After that it checks if in the MySQL table already contains any row in the same location

If it doesn't it will insert the row, if it does it will delete and insert the row.

This function is activated every time a new item is add to the grid no matter if it exists or not, if it exists it will just change the quantity.

I want to know if there is anyway I can make this function faster because it is taking to much time, about 2 seconds.

Answer 1

With this code:

string items = "";
 for (int i = 0; i < _ProductsGrid.Rows.Count; i++)
 {
 items += _ProductsGrid.Rows[i].Cells[1].Value.ToString();
 items += ",";
 }
items = items.Remove(items.Length - 1, 1);

You're concatenating a list of values into a string. Let me first say a word about that try/catch. Exceptions are exceptional conditions, if you can avoid an exception then you should:

if (items.Length > 1)
 items = items.Remove(items.Length - 1, 1);

However you do not need it, both items and Qnt can be created with like this (assuming _ProductsGrid is a DataGrid but same logic applies in any case):

var rows = _ProductsGrid.Rows.Cast<DataRow>();
string items String.Join(";",
 rows.Select(x => Convert.ToString(x.Cells[1].Value)));

---

Now let's see you SQL. You're building SQL commands into strings, it makes your code hard to debug, open to SQL injection attacks and fragile because invalid and unexpected user inputs will crash the application. Use SQL parameters:

string query = "SELECT * FROM TempSave WHERE Localization=@Localization";

Now you have to build the command:

using (var cmd = new MySqlCommand(query, connection))
{
 cmd.Parameters.AddWithValue("@Localization", _Save.Localization);
 // ...
}

And you have the reader:

using (var reader = cmd.ExecuteReader())
{
 // ...
}

Note that using parameters also the string concatenation will go away (because you can dynamically create named parameters as @param1, @param2 and so on).

---

You do not need to call MySqlConnection.Close() if (correctly!) you're already wrapped the object inside a using statement.

---

Please follow usual C# naming guidelines, they will make your code easier to read for everyone (for example local variables should be camelCase). Stick to one convention and follow it (to have items and Qnt is confusing, why they have different casing?)

Also try to use meaningful names for your variables. Code should be a fluent sentence to read like a book.

---

Catch exceptions. Things may go wrong and you do not want to crash entire application.

---

Validate user input. What if user didn't enter anything in one of your input boxes?

---

Split code into separate methods, it helps because to understand whole logic you need to read just few lines, not the entire code. Think (names are weird because I do not know your domain but you must do better):

if (CheackTemDataAlreadyExists())
 DeleteCheackTemData();
InsertCheackTemData();

Note that you should avoid code duplication (INSERT was repeated twice).

---

Do not hard-code constants (like connection string). Ideally it should be in your configuration file but, at least, move it to a private const string field.

Answer 2

String is immutable so you should not use it in a loop.

StringBuilder sb = new StringBuilder();
for (int i = 0; i < _ProductsGrid.Rows.Count; i++)
{
 if (sb.Length > 0)
 {
 sb.Append(",")
 }
 sb.Append(_ProductsGrid.Rows[i].Cells[1].Value.ToString());
} 

Don't open and close the connection multiple times. Open and close once.

Why is _CheackTem an IEnumerable when you only need the count?

Why are you delete and insert rather than update?

Qnt should not be capital. Why _S?

Why are you looping the same grid twice. This code has some problems.

This should be faster and cleaner.

public void SaveProducts()
{ 
 string localization = _LocationLabel.Text + "," + _LocNumbLab.Text;
 StringBuilder sbItems = new StringBuilder();
 StringBuilder sbQty = new StringBuilder();
 for (int i = 0; i < _ProductsGrid.Rows.Count; i++)
 {
 if (sbItems.Length > 0)
 {
 sbItems.Append(",")
 }
 sbItems.Append(_ProductsGrid.Rows[i].Cells[1].Value.ToString());
 if (sbQty.Length > 0)
 {
 sbQty.Append(",")
 }
 sbQty.Append(_ProductsGrid.Rows[i].Cells[0].Value.ToString());
 } 
 string items = sbItems.ToString();
 string qty = sbQty.ToString();
 using (MySqlConnection Con = new MySqlConnection("server=localhost; database=wintestbeta;user=root; password=;"))
 {
 Con.Open();
 string query = "SELECT count(*) FROM TempSave WHERE Localization='" + _Save.Localization + "'";
 int count = Con.Query(query); //I don't know mysql syntax
 if (_count == 0)
 {
 query = "INSERT INTO TempSave (Localization,items,Quantity) 
 VALUES('" + localization + "','" +items + "','" + qty + "')";
 Con.Query(query);
 }
 else
 {
 //replace with update
 query = "DELETE FROM TempSave WHERE Localization='" + localization + "'";
 Con.Query(query);
 query = "INSERT INTO TempSave (Localization,items,Quantity) 
 VALUES('" + localization + "','" + items + "','" + qty+ "')";
 Con.Query(query);
 }
 }
}

• Bot Wade: _CheackTem is a IEnumerable because Con.Query can only return IEnumerables, the update is a good idea
• paparazzo: I am having a hard time believing you cannot get a count in MySql
• Bot Wade: i am using Dapper
• paparazzo: Why are you using Dapper to get a simple count? You asked how to make it go faster.
• Bot Wade: this is not the entire code there are a lot of more things