TransWikia.com

C# Making Multiple else-if statements more efficient. Methods?

Stack Overflow Asked by Kajin on November 24, 2021

private void displayButton_Click(object sender, EventArgs e)
    {


        string connectionString;
        SqlConnection cnn;

        connectionString = "Server= myservername; Database= nba_database; Integrated Security=True";

        cnn = new SqlConnection(connectionString);

        cnn.Open();
       // MessageBox.Show("Connection Established!");

   

        // define variables
        SqlCommand command;
        
        String sql;

        // define SQL statement
        if (listBox1.SelectedIndex == 0)
        {
            sql = "SELECT LastName, FirstName " +
            "FROM Players, Teams " +
            "WHERE Players.TeamID = Teams.TeamID " +
            "AND Teams.Nickname = 'Hawks'";

            // command statement
            command = new SqlCommand(sql, cnn);


            textBox1.Text = command.ExecuteScalar().ToString();
            // Scalar returns the first column of the first row of the query


            cnn.Close();
            
            command.Dispose();
        }
        else if (listBox1.SelectedIndex == 1)
        {
            String sql2 = "SELECT FirstName, LastName FROM Players WHERE Team = 'Milwaukee Bucks'" +
                            "AND Number <= 24";

            

            // command statement
            command = new SqlCommand(sql2, cnn);

            SqlDataReader reader = command.ExecuteReader();


            // Get table values

            StringBuilder sb = new StringBuilder();
            while (reader.Read())
            {
                sb.AppendLine(reader.GetString(0).ToString() + " " + reader.GetString(1).ToString());
            }

            textBox1.Text = sb.ToString();


            cnn.Close();

            command.Dispose();
        }   

    }

I have this code that connects to a SQL Server database. There is a listbox with options for the user to select, and based on their selection, a query will run and return the query results in a textbox.

As it stands, it feels like I have a bunch of repetitive code and I only have 2 options in the list box. I will need to add more options and the nested if else statements will just keep growing.

Is there a better way to do this? I was thinking of using methods but when I tried that I couldn’t figure out how to return the value from a method and then how to use that as a parameter for my query.

Any help/advice will be much appreciated

2 Answers

I would do something like this.

But rename SelectIndexZero/SelectIndexOne to something more describing.

And i would keep the IF statement, don't change to case because you don't have a default case. Only 2 very specific cases. If 0 is a default value, I would change it to a case.

private void displayButton_Click(object sender, EventArgs e)
{
    string connectionString = "Server= myservername; Database= nba_database; Integrated Security=True";

    SqlConnection cnn = new SqlConnection(connectionString);

    cnn.Open();
    // MessageBox.Show("Connection Established!");

    // define variables
    SqlCommand command;
    
    // define SQL statement
    if (listBox1.SelectedIndex == 0)
    {
        SelectIndexZero(cnn, command);
    }
    else if (listBox1.SelectedIndex == 1)
    {
        SelectIndexOne(cnn, command);
    }

    Dispose(cnn, command);
}

private void SelectIndexZero(SqlConnection cnn, SqlCommand command)
{
    string sql = "SELECT LastName, FirstName " +
        "FROM Players, Teams " +
        "WHERE Players.TeamID = Teams.TeamID " +
        "AND Teams.Nickname = 'Hawks'";

    // command statement
    command = new SqlCommand(sql, cnn);

    textBox1.Text = command.ExecuteScalar().ToString();
    // Scalar returns the first column of the first row of the query
}  

private void SelectIndexOne(SqlConnection cnn, SqlCommand command)
{
    string sql = "SELECT FirstName, LastName FROM Players WHERE Team = 'Milwaukee Bucks'" +
                        "AND Number <= 24";

    // command statement
    command = new SqlCommand(sql, cnn);

    SqlDataReader reader = command.ExecuteReader();


    // Get table values

    StringBuilder sb = new StringBuilder();
    while (reader.Read())
    {
        sb.AppendLine(reader.GetString(0).ToString() + " " + reader.GetString(1).ToString());
    }

    textBox1.Text = sb.ToString();
}

private void Dispose(SqlConnection cnn, SqlCommand command)
{
    cnn.Close();
    command.Dispose();
}

Answered by Eric Sjöström on November 24, 2021

Well, initially the switch statement will tidy it up:

   var i = listbox1.SelectedIndex;

   switch (i)
    {
        case 1:
            // do stuff
            break;
        case 2:
            // do other stuff
            break;
        default:
            break;
    }

But this will quickly become unwieldy too.

Answered by Alan B on November 24, 2021

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP