4
\$\begingroup\$

I have a combobox with two items. And if one is selected I want to write data to "salary" table and if selected another, to "other" table.

The difference between two code blocks is only one word. I would like to know how I could avoid repeating same code twice.

string myConnection = "datasource=localhost; port=3306; username=root;password=root";
                if (comboBox4.SelectedItem.ToString() == "Salary")
                {
                    string insert = "insert into budget.salary (name, suma) values (@name, @price);";

                    using (var conDataBase = new MySqlConnection(myConnection))
                    using (var cmdDataBase = new MySqlCommand(insert, conDataBase))
                    {
                        cmdDataBase.Parameters.AddWithValue("@name", name);
                        cmdDataBase.Parameters.AddWithValue("@price", suma);
                        conDataBase.Open();
                        cmdDataBase.ExecuteNonQuery();

                        MessageBox.Show("Saved");
                    }
                }
                if (comboBox4.SelectedItem.ToString() == "Other")
                {
                    string insert = "insert into budget.other (name, suma) values (@name, @price);";

                    using (var conDataBase = new MySqlConnection(myConnection))
                    using (var cmdDataBase = new MySqlCommand(insert, conDataBase))
                    {
                        cmdDataBase.Parameters.AddWithValue("@name", name);
                        cmdDataBase.Parameters.AddWithValue("@price", suma);
                        conDataBase.Open();
                        cmdDataBase.ExecuteNonQuery();
                        MessageBox.Show("Saved");

                    }
                }
\$\endgroup\$
2
  • \$\begingroup\$ I don't actually see the variable insert used anywhere! \$\endgroup\$
    – abelenky
    Commented Jul 10, 2016 at 0:00
  • \$\begingroup\$ It just, variables was written in my native language and i was changing to english. it seems, that i missed some. I will correct that. But i get my answers, a lot of good information. and things to learn about, thank you. \$\endgroup\$
    – Harikeinas
    Commented Jul 10, 2016 at 0:10

6 Answers 6

4
\$\begingroup\$

Declare a method:

public void InsertInto(string table) {
  string myConnection = "datasource=localhost; port=3306; username=root;password=root";
  string insert = "insert into budget." + table + " (name, suma) values (@name, @price);";

  using (var conDataBase = new MySqlConnection(myConnection))
  using (var cmdDataBase = new MySqlCommand(iterpti, conDataBase)) {
    cmdDataBase.Parameters.AddWithValue("@name", name);
    cmdDataBase.Parameters.AddWithValue("@price", suma);
    conDataBase.Open();
    cmdDataBase.ExecuteNonQuery();
    MessageBox.Show("Saved");
  }
}

And change your code to call the method:

if (comboBox4.SelectedItem.ToString() == "Salary") {
    InsertInto("salary");
}
if (comboBox4.SelectedItem.ToString() == "Other") {
    InsertInto("other");     
}

Declaring a method that does what you want and then calling the method multiple times instead of repeating your code multiple times is the standard way to avoid duplicating code. This principle is known as encapsulation and is very necessary for writing clean, readable, and reusable code. You should get used to using this technique whenever possible.

More info: https://en.wikipedia.org/wiki/Encapsulation_(computer_programming)

\$\endgroup\$
4
\$\begingroup\$

If the difference is only one word, your if statement should just change that one word. Something like this:

string tableName = null; // tableName is the only difference between the two, right?
if (comboBox4.SelectedItem.ToString() == "Salary") {
    tableName = "salary";
} else if (comboBox4.SelectedItem.ToString() == "Other") {
    tableName = "other";
}

I suggest you to also add an else clause so that when the user selected neither of these, you can have some default table to insert into. But I'll assume you don't have one at the moment.

Now check whether tableName is null. If it isn't, do the insertion and stuff:

if (tableName != null) {
    string iterpti = $"insert into budget.{tableName} (name, suma) values (@name, @price);";

    using (var conDataBase = new MySqlConnection(myConnection))
    using (var cmdDataBase = new MySqlCommand(iterpti, conDataBase))
    {
        cmdDataBase.Parameters.AddWithValue("@name", name);
        cmdDataBase.Parameters.AddWithValue("@price", suma);
        conDataBase.Open();
        cmdDataBase.ExecuteNonQuery();

        MessageBox.Show("Saved");
    }

}

You can also extract the insertion thingy as a method. You can easily do this by using the "Extract Method" feature in Visual Studio.

private void InsertIntoTable(string tableName) {
    string iterpti = $"insert into budget.{tableName} (name, suma) values (@name, @price);";

    using (var conDataBase = new MySqlConnection(myConnection))
    using (var cmdDataBase = new MySqlCommand(iterpti, conDataBase))
    {
        cmdDataBase.Parameters.AddWithValue("@name", name);
        cmdDataBase.Parameters.AddWithValue("@price", suma);
        conDataBase.Open();
        cmdDataBase.ExecuteNonQuery();

        MessageBox.Show("Saved");
    }
}

If you're not using C# 6, you can't use string interpolation. You have to manually add the strings together:

string iterpti = "insert into budget." + tableName + " (name, suma) values (@name, @price);";
\$\endgroup\$
1
\$\begingroup\$

Create a method which does the insertion to the table and have it accept a parameter of type string which you should pass while calling comboBox4.SelectedItem.ToString() like

private void DoInsertion(string tableName)
{
  //logic to insert data
}

Call it like

                if (comboBox4.SelectedItem.ToString() == "Salary")
                {
                    DoInsertion("Salary");
                }
\$\endgroup\$
0
1
\$\begingroup\$

This is one of the many suggestions. Feel free to decide whichever works the best for you. :-)

Option 1

string myConnection = "datasource=localhost; port=3306; username=root;password=root";
string value = comboBox4.SelectedItem.ToString();
string insert = "";    

switch (value)
{
    case "Salary":
        insert = "insert into budget.salary (name, suma) values (@name, @price);";
        break;

    case "Other":
        insert = "insert into budget.other (name, suma) values (@name, @price);";
        break;

    # This case is optional in this situation.
    # You may not have to use this.
    default:
        break;
}

using (var conDataBase = new MySqlConnection(myConnection))
using (var cmdDataBase = new MySqlCommand(insert, conDataBase))
{
    cmdDataBase.Parameters.AddWithValue("@name", name);
    cmdDataBase.Parameters.AddWithValue("@price", suma);
    conDataBase.Open();
    cmdDataBase.ExecuteNonQuery();

    MessageBox.Show("Saved");
}

This uses the Switch case statement, which is very similar to If / Else If, Else conditions. Tutorials Point has a good guide for switch case.

Option 2

Alternatively, you could use the If / Else If / Else conditions like you did in your post, except with some changes. Like so:

string myConnection = "datasource=localhost; port=3306; username=root;password=root";
string value = comboBox4.SelectedItem.ToString().ToLower();
string insert = "";    

if (value == "salary")
{
        insert = "insert into budget.salary (name, suma) values (@name, @price);";
}

else if (value == "other")
        insert = "insert into budget.other (name, suma) values (@name, @price);";
}

using (var conDataBase = new MySqlConnection(myConnection))
using (var cmdDataBase = new MySqlCommand(insert, conDataBase))
{
    cmdDataBase.Parameters.AddWithValue("@name", name);
    cmdDataBase.Parameters.AddWithValue("@price", suma);
    conDataBase.Open();
    cmdDataBase.ExecuteNonQuery();

    MessageBox.Show("Saved");
}

I only left the insert = "Some SQL statement"; in the If / Else If block because it is the only thing that depends on what the value string is. The If statements should be used wisely to avoid unnecessary redundancy in your code. I currently can't find any source for you to read on this, but I will add it to this answer as soon as I find it.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you :). I do use if very often, and know my 50 line code is kinda uglys with if{if{if{}}} just for inserting data to mysql. :D one is if (comboBox4.SelectedItem != null) another if (decimal.TryParse(price.Replace(",","."), out suma)) and last one ` if (comboBox4.SelectedItem.ToString() == "salary")` well not just for inserting data, more for input control. \$\endgroup\$
    – Harikeinas
    Commented Jul 10, 2016 at 3:19
  • \$\begingroup\$ No worries. @nhouser9 's answer is the better way to avoid code redundancy, though. I would recommend following his or her method :) \$\endgroup\$ Commented Jul 11, 2016 at 0:47
0
\$\begingroup\$

This uses string interpolation to keep it as simple as I possibly can.

string myConnection = "datasource=localhost; port=3306; username=root;password=root";
string budgetField = comboBox4.SelectedItem.ToString().ToLower(); // This will be either "salary" or "other"

// This is where the magic happens:  The {bracketed} variable gets repaced with either "salary" or "other".
string insert = $"insert into budget.{budgetField} (name, suma) values (@name, @price);";

using (var conDataBase = new MySqlConnection(myConnection))
using (var cmdDataBase = new MySqlCommand(iterpti, conDataBase))
{
    cmdDataBase.Parameters.AddWithValue("@name", name);
    cmdDataBase.Parameters.AddWithValue("@price", suma);
    conDataBase.Open();
    cmdDataBase.ExecuteNonQuery();
    MessageBox.Show("Saved");
}
\$\endgroup\$
0
0
\$\begingroup\$

I have not tested this code. But here you go with my implementation. I would be happy to hear comments from the community.

public class Example1
{
    private const string ConnectionString = "datasource=localhost; port=3306; username=root;password=root";

    public void ExecuteOperation(string valueCombobox, string name, double price)
    {
        var sql = ConstructSql(valueCombobox);
        var value = InsertOperation(ConnectionString, sql, name, price);

        //Do other things overhere like display the success message since you have the value variable.
    }

    private string ConstructSql(string valueCombobox)
    {
        return string.Format("insert into budget.{0} (name, suma) values (@name, @price)", valueCombobox);
    }
    private int InsertOperation(string connectionstring, string sql, string name, double price)
    {
        using (var connection = new MySqlConnection(connectionstring))
        using (var command = new MySqlCommand(sql, connection))
        {
            command.Parameters.AddWithValue("@name", name);
            command.Parameters.AddWithValue("@price", price);
            connection.Open();

            return command.ExecuteNonQuery();
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ Commented Jan 9, 2019 at 9:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.