-2
\$\begingroup\$

Background: I'm designing a system Data entry form using c# as front end and sql server 2008 as backend database. In this app I'm basically, just reading/writing to the database

My issue:

  1. I never truly understood OOP, however I am getting more involved with it now, and while my program works, I want the code to be better (look better, be more flexible, utilize classes/objects more, etc.).
  2. Being my first application I have no idea how messy the code is, if it conforms to guidelines, etc. Hopefully I will get some good feedback about improvements that can be made, but other than that, it works perfectly in all my tests!
  3. I have written all the functionality in a single class with my understanding it is not a correct way to do. But how to know whether which functionality should be separated or aggregated

Note: The code below is a fully working application where all the business case has been working fine.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Data.SqlClient;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Data.Sql;
using System.Windows.Forms;

namespace WindowsFormsApplication2
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }


        private void textBox1_TextChanged(object sender, EventArgs e)
        {
//Wrote the code to populate the date time on the time of loding the form

            DateTime dateTime = DateTime.Now;

            try
            {
                DateTime dt = DateTime.Parse(textBox1.Text);


//The user should not select the time that is greater than the current date time
                if (dt > dateTime )
                {
                    MessageBox.Show("Dateshould not be greater than current date");
                    textBox1.Text = dateTime.ToString("yyyy-MM-dd");
                }
                else if( dt <= dateTime.AddDays(-16))
                {
                    MessageBox.Show("Date Should Not Be Less Than 15days From The Current Date");
                    textBox1.Text = dateTime.ToString("yyyy-MM-dd");
                }
            }
            catch (Exception ex)
            {

            }
        }



        private void Form1_Load_1(object sender, EventArgs e)
        {
            /*The text should not be editable for the following textboes 
             1) Date
             2) username
             3) Total Audits Reviewed
             4) Audit carry forward
             5) Pending*/

            textBox1.ReadOnly = true;
            textBox9.ReadOnly = true;
            textBox12.ReadOnly = true;
            textBox8.ReadOnly = true;

            this.FormBorderStyle = FormBorderStyle.FixedSingle;
            this.MaximizeBox = false;
            this.MinimizeBox = true;
            dateTimePicker1.Visible = false;
            textBox1.Text = dateTimePicker1.Value.ToString("yyyy-MM-dd");
            textBox11.Text = Environment.UserName;

            dropdown_combox1_values();
            dropdown_combox5_values();
            button2.Enabled = false;
            button3.Enabled = false;
            comboBox3.SelectedIndex = 4;
        }
        private void dateTimePicker1_ValueChanged(object sender, EventArgs e)
        {
            textBox1.Text = dateTimePicker1.Value.ToString("yyyy-MM-dd");
            dateTimePicker1.Visible = false;
        }


        private void textBox1_DoubleClicked(object sender, EventArgs e)
        {
            dateTimePicker1.Visible = true;
        }

        public void dropdown_combox1_values()
        {
            comboBox1.Items.Add("Home Health - Post pay");
            comboBox1.Items.Add("Home Health - Pre pay");
        }

        public void dropdown_combox5_values()
        {
            comboBox3.Items.Add("OASIS");
            comboBox3.Items.Add("Per visit");
            comboBox3.Items.Add("Leave");
            comboBox3.Items.Add("Holiday");

            comboBox3.Items.Add("N\\A");
            comboBox3.Items.Add("QC");

            comboBox3.DropDownStyle = ComboBoxStyle.DropDownList;
        }

        public void dropdown_combox2_values()
        {
            comboBox2.Items.Add("Humana OASIS");
            comboBox2.Items.Add("Humana Per visit");
        }

        private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
        {
            comboBox1.DropDownStyle = ComboBoxStyle.DropDownList;
            comboBox2.Items.Clear();

            dropdown_combox2_values();

        }

        private void comboBox2_SelectedIndexChanged_1(object sender, EventArgs e)
        {
            comboBox2.DropDownStyle = ComboBoxStyle.DropDownList;
        }

        private void button1_Click(object sender, EventArgs e)
        {
            int i;
            /*
             Check for test condition when the user will give data that violates the process
             */

            if (comboBox1.SelectedIndex != -1)
            {
                if (comboBox2.SelectedIndex != -1)
                {

                    if (textBox2.Text != "")
                    {
                        if (int.TryParse(textBox2.Text, out i))
                        {
                            if (textBox3.Text != "")
                            {
                                if (int.TryParse(textBox3.Text, out i))
                                {
                                    if (textBox4.Text != "")
                                    {
                                        if (int.TryParse(textBox4.Text, out i))
                                        {
                                            if (textBox5.Text != "")
                                            {
                                                if (int.TryParse(textBox5.Text, out i))
                                                {

                                                    if(textBox6.Text != "" )
                                                    {
                                                        if(int.TryParse(textBox6.Text, out i))
                                                        {
                                                            if(textBox7.Text != "")
                                                            {
                                                                if (int.TryParse(textBox7.Text, out i))
                                                                {
                                                                    if (textBox10.Text != "")
                                                                    {
                                                                        if(int.TryParse(textBox10.Text, out i))
                                                                        {
                                                                            if (comboBox3.SelectedIndex != -1)
                                                                            {
                                                                                calcuate_values();
                                                                            }
                                                                            else
                                                                            {
                                                                                MessageBox.Show("Please select the Appropriate Comment", "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);

                                                                            }



                                                                        }
                                                                        else
                                                                        {
                                                                            MessageBox.Show("Check whether the data is a number",
                                                    "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                                            textBox10.Text = "";
                                                                        }
                                                                    }
                                                                    else
                                                                    {

                                                                        MessageBox.Show("Provide Audits Assigned","Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                                    }


                                                                }
                                                                else
                                                                {
                                                                    MessageBox.Show("Check whether the data is a number",
                                                            "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                                    textBox7.Text = "";

                                                                }
                                                            }
                                                            else
                                                            {
                                                                MessageBox.Show("Provide Target/day",
                                                          "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                            }

                                                        }
                                                        else
                                                        {
                                                            MessageBox.Show("Check whether the data is a number",
                                                            "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                            textBox6.Text = "";
                                                        }
                                                    }
                                                    else
                                                    {
                                                        MessageBox.Show("Provide Unfullfilled Data",
                                                            "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                    }

                                                }
                                                else
                                                {
                                                    MessageBox.Show("Check whether the data is a number", "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                                    textBox5.Text = "";

                                                }

                                            }
                                            else
                                            {
                                                MessageBox.Show("Provide Rejection Data", "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);


                                            }

                                        }
                                        else
                                        {
                                            MessageBox.Show("Check whether the data is a number", "Error", MessageBoxButtons.OK,
MessageBoxIcon.Warning);
                                            textBox4.Text = "";
                                        }
                                    }
                                    else
                                    {
                                        MessageBox.Show("Provide Total Findings Data", "Error", MessageBoxButtons.OK,
                                                         MessageBoxIcon.Warning);
                                    }

                                }
                                else
                                {
                                    MessageBox.Show("Check whether the data is a number", "Error", MessageBoxButtons.OK,
                           MessageBoxIcon.Warning);
                                    textBox3.Text = "";

                                }

                            }
                            else
                            {
                                MessageBox.Show("Provide Partital Denial Data", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);
                            }
                        }
                        else
                        {
                            MessageBox.Show("Check whether the data is a number", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);
                            textBox2.Text = "";
                        }

                    }
                    else
                    {
                        MessageBox.Show("Provide Full Denial Data", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);
                    }
                }
                else
                {
                    MessageBox.Show("Select Workflow", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);

                }
            }
            else
            {
                MessageBox.Show("Select Program", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);
            }

        }

        private void textBox11_TextChanged(object sender, EventArgs e)
        {
            textBox11.ReadOnly = true;
        }

        private void comboBox3_SelectedIndexChanged(object sender, EventArgs e)
        {

        }

        //private void textBox_TextChanged(object sender, EventArgs e)
        //{


        //}


        public void calcuate_values()
        {

            try
            {

                var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MYConnectionString"].ConnectionString;

                using (var connection = new SqlConnection(connectionString))
                {
                    connection.Open();
                    using (var command = new SqlCommand(@"[NAS\kalais1].[HomeHealth_Validate_Search]", connection))
                    {
                        command.CommandType = CommandType.StoredProcedure;
                        command.Parameters.Add("@Date", SqlDbType.Date).Value = textBox1.Text;
                        command.Parameters.Add("@Username", SqlDbType.VarChar).Value = textBox11.Text;
                        command.Parameters.Add("@workflow", SqlDbType.VarChar).Value = comboBox2.Text;
                        command.Parameters.Add("@program", SqlDbType.VarChar).Value = comboBox1.Text;
                        SqlParameter returnParameter = command.Parameters.Add("RetVal", SqlDbType.Int);
                        returnParameter.Direction = ParameterDirection.ReturnValue;
                        command.ExecuteNonQuery();

                        SqlDataReader oReader = command.ExecuteReader();

                            while (oReader.Read())
                            {
                                if (oReader["Pending_Audits"].ToString() == "Already exists")
                                {
                                    MessageBox.Show("The given workflow and program is already present for the given date");
                                    textBox12.Text = "";
                                    textBox8.Text = "";
                                    textBox9.Text = "";
                                }
                                else
                                {
                                    textBox12.Text =
                                                                                    (Convert.ToInt32(textBox2.Text) +
                                                                                     Convert.ToInt32(textBox3.Text) +
                                                                                     Convert.ToInt32(textBox4.Text)).ToString();
                                    textBox8.Text = oReader["Pending_Audits"].ToString();
                                    textBox9.Text =
                                        ((Convert.ToInt32(textBox10.Text) + Convert.ToInt32(textBox8.Text)) -
                                         Convert.ToInt32(textBox12.Text)).ToString();
                                    MessageBox.Show("Validated the data! Please Submit.. ");

                                    button2.Enabled = true;
                                }
                    }
             }
                }
            }
            catch (Exception ex)
            {
                // Print error message
                MessageBox.Show(ex.Message);
            }
        }

        private void textBox10_TextChanged(object sender, EventArgs e)
        {

        }

        private void Show_Click(object sender, EventArgs e)
        {
            Form2 f2 = new Form2();
            f2.ShowDialog();


        }
        public void ClearTextBoxes(Control control)
        {
            foreach (Control c in control.Controls)
            {
                if (c is TextBox)
                {
                    if (c.Name != "textBox11" && c.Name != "textBox1")
                    {
                        if (!(c.Parent is NumericUpDown))
                        {
                            ((TextBox) c).Clear();
                        }
                    }
                }
               else if (c is ComboBox)
                {
                    ((ComboBox)c).SelectedIndex = -1;
                    comboBox3.SelectedIndex = 4;
                }

                if (c.HasChildren)
                {
                    ClearTextBoxes(c);
                }
            }
        }
        private void button2_Click(object sender, EventArgs e)
        {
            var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MYConnectionString"].ConnectionString;

            using (var connection = new SqlConnection(connectionString))
            {
                connection.Open();
                using (var command = new SqlCommand(@"[NAS\kalais1].[HomeHealth_Submit_test]", connection))
                {

                    command.CommandType = CommandType.StoredProcedure;
                    command.Parameters.Add("@Date", SqlDbType.Date).Value = textBox1.Text;
                    command.Parameters.Add("@Username", SqlDbType.VarChar).Value = textBox11.Text;
                    command.Parameters.Add("@workflow", SqlDbType.VarChar).Value = comboBox2.Text;
                    command.Parameters.Add("@program", SqlDbType.VarChar).Value = comboBox1.Text;
                    command.Parameters.Add("@Full_Denial", SqlDbType.Int).Value = textBox2.Text;
                    command.Parameters.Add("@Partial_Denial", SqlDbType.Int).Value = textBox3.Text;
                    command.Parameters.Add("@No_Findings", SqlDbType.Int).Value = textBox4.Text;
                    command.Parameters.Add("@Rejections", SqlDbType.Int).Value = textBox5.Text;
                    command.Parameters.Add("@Unfulfilled", SqlDbType.Int).Value = textBox6.Text;
                    command.Parameters.Add("@TargetPerDay", SqlDbType.Int).Value = textBox7.Text;
                    command.Parameters.Add("@Audits_Assigned", SqlDbType.Int).Value = textBox10.Text;
                    command.Parameters.Add("@Total_Audits_Reviewed", SqlDbType.Int).Value = textBox12.Text;
                    command.Parameters.Add("@Audits_Carry_Forward", SqlDbType.Int).Value = textBox8.Text;
                    command.Parameters.Add("@Pending_Audits", SqlDbType.Int).Value = textBox9.Text;
                    command.Parameters.Add("@Comments", SqlDbType.VarChar).Value = comboBox3.Text;

                    SqlParameter returnParameter = command.Parameters.Add("RetVal", SqlDbType.Int);
                    returnParameter.Direction = ParameterDirection.ReturnValue;


                    using (SqlDataReader oReader = command.ExecuteReader())
                    {
                        while (oReader.Read())
                        {
                            if (oReader["comments1"].ToString() == "0")
                            {
                                MessageBox.Show("CHECK THE COMMENTS AND THE GIVEN VALUE");

                            }
                            else
                            {
                                MessageBox.Show("Inserted");
                                ClearTextBoxes(this);
                                button2.Enabled = false;
                            }



                        }
                    }

                }
            }
        }

        private void Search_Click(object sender, EventArgs e)
        {
            button1.Enabled = false;
            button2.Enabled = false;

            if (comboBox1.SelectedIndex != -1)
            {
                if (comboBox2.SelectedIndex != -1)
                {
                    var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MYConnectionString"].ConnectionString;

                    using (var connection = new SqlConnection(connectionString))
                    {
                connection.Open();
                using (var command = new SqlCommand(@"[NAS\kalais1].[HomeHealth_Search]", connection))
                {
                    command.CommandType = CommandType.StoredProcedure;
                    command.Parameters.Add("@Workflow", SqlDbType.VarChar).Value = comboBox2.Text;
                    command.Parameters.Add("@Date", SqlDbType.Date).Value = textBox1.Text;
                    command.Parameters.Add("@Username", SqlDbType.VarChar).Value = textBox11.Text;
                    command.Parameters.Add("@program", SqlDbType.VarChar).Value = comboBox1.Text;
                    SqlParameter returnParameter = command.Parameters.Add("RetVal", SqlDbType.Int);
                    returnParameter.Direction = ParameterDirection.ReturnValue;
                   // command.ExecuteNonQuery();

                    //MessageBox.Show(returnParameter.Value.ToString());

                    //MessageBox.Show(comboBox2.Text);

                    using (SqlDataReader oReader = command.ExecuteReader())
                    {
                        while (oReader.Read())
                        {
                            if (oReader["Error"].ToString() == "0")
                            {

                                textBox2.Text = oReader["Full_Denial"].ToString();
                                textBox3.Text = oReader["Partial_Denial"].ToString();
                                textBox4.Text = oReader["No_Findings"].ToString();
                                textBox5.Text = oReader["Rejections"].ToString();
                                textBox6.Text = oReader["Unfulfilled"].ToString();
                                textBox7.Text = oReader["TargetPerDay"].ToString();
                                textBox10.Text = oReader["Audits_Assigned"].ToString();
                                comboBox3.Text = oReader["Comments"].ToString();
                                textBox12.Text = oReader["Total_Audits_Reviewed"].ToString();
                                textBox8.Text = oReader["Audits_Carry_Forward"].ToString();
                                textBox9.Text = oReader["Pending_Audits"].ToString();
                                MessageBox.Show("Required Data... Has been populated");
                                button3.Enabled = true;
                                comboBox1.Enabled = false;
                                comboBox2.Enabled = false;

                            }
                            else
                            {
                                MessageBox.Show("Check Whether You have selected Program and workflow currectly");
                                ClearTextBoxes(this);
                                comboBox1.Enabled = true;

                                comboBox2.Enabled = true;
                                button1.Enabled = true;

                            }


                        }

                    }
                }

            }

                }
                else
                {
                MessageBox.Show("Select Workflow", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);

                }
            }
            else
            {
                MessageBox.Show("Select Program", "Error", MessageBoxButtons.OK,
                                                       MessageBoxIcon.Warning);
            }


        }

        private void button3_Click(object sender, EventArgs e)
        {
            textBox12.Text =
                                                                                   (Convert.ToInt32(textBox2.Text) +
                                                                                    Convert.ToInt32(textBox3.Text) +
                                                                                    Convert.ToInt32(textBox4.Text)).ToString();

            textBox9.Text =
                                       ((Convert.ToInt32(textBox10.Text) + Convert.ToInt32(textBox8.Text)) -
                                        Convert.ToInt32(textBox12.Text)).ToString();



            var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MYConnectionString"].ConnectionString;

            using (var connection = new SqlConnection(connectionString))
            {
                connection.Open();
                using (var command = new SqlCommand(@"[NAS\kalais1].[HomeHealth_Update]", connection))
                {

                    command.CommandType = CommandType.StoredProcedure;
                    command.Parameters.Add("@Workflow", SqlDbType.VarChar).Value = comboBox2.Text;
                    command.Parameters.Add("@date", SqlDbType.Date).Value = textBox1.Text;
                    command.Parameters.Add("@Username", SqlDbType.VarChar).Value = textBox11.Text;
                    command.Parameters.Add("@Program", SqlDbType.VarChar).Value = comboBox1.Text;
                    command.Parameters.Add("@Full_Denial", SqlDbType.Int).Value = textBox2.Text;
                    command.Parameters.Add("@Partial_Denial", SqlDbType.Int).Value = textBox3.Text;
                    command.Parameters.Add("@No_Findings", SqlDbType.Int).Value = textBox4.Text;
                    command.Parameters.Add("@Rejections", SqlDbType.Int).Value = textBox5.Text;
                    command.Parameters.Add("@Unfulfilled", SqlDbType.Int).Value = textBox6.Text;
                    command.Parameters.Add("@TargetPerDay", SqlDbType.Int).Value = textBox7.Text;
                    command.Parameters.Add("@Audits_Assigned", SqlDbType.Int).Value = textBox10.Text;
                    command.Parameters.Add("@Total_Audits_Reviewed", SqlDbType.Int).Value = textBox12.Text;
                    command.Parameters.Add("@Audits_Carry_Forward", SqlDbType.Int).Value = textBox8.Text;
                    command.Parameters.Add("@Pending_Audits", SqlDbType.Int).Value = textBox9.Text;
                    command.Parameters.Add("@Comments", SqlDbType.VarChar).Value = comboBox3.Text;

                    SqlParameter returnParameter = command.Parameters.Add("RetVal", SqlDbType.Int);
                    returnParameter.Direction = ParameterDirection.ReturnValue;


                    using (SqlDataReader oReader = command.ExecuteReader())
                    {
                        while (oReader.Read())
                        {
                            if (oReader["comments1"].ToString() == "0")
                            {
                                MessageBox.Show("Please.. Check the comments and populated values");

                            }
                            else
                            {
                                MessageBox.Show("Updated");
                                ClearTextBoxes(this);
                                comboBox1.Enabled = true;

                                comboBox2.Enabled = true;
                                button1.Enabled = true;
                                button2.Enabled = false;
                                button3.Enabled = false;

                            }



                        }
                    }

                     }
            }


            }



    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Please describe what your application is doing in more detail. It'd be great if you could also share a screenshot. \$\endgroup\$ – t3chb0t Sep 17 '18 at 14:33
1
\$\begingroup\$

Some quick remarks

  • Name you things properly. E.g comboBox3 will make you wonder what it is, if you come in 2 months to fix a bug or add some functionality. Always name your things in a way that you see at first glance what they are about.
  • Instead of using DateTime.Parse() inside a try..catch where you swallow every exception you either should use DateTime.TryParse() or catch the FormatException which the method would throw. But not only catching you should but for a gui you should also inform the user that something went wrong.
  • Inside the button1_Click() method you could avoid the Arrow Anti Pattern which makes your code almost unreadable, by returning early like e.g

    if (comboBox1.SelectedIndex != -1)
    {
        MessageBox.Show("Select Program", "Error", MessageBoxButtons.OK, MessageBoxIcon.Warning);
        return;
    }
    if (comboBox2.SelectedIndex != -1)
    {
        MessageBox.Show("Select Workflow", "Error", MessageBoxButtons.OK, MessageBoxIcon.Warning);
        return ;
    }  
    
    ....  
    
  • commented out code is dead code which should be removed like e.g

    //private void textBox_TextChanged(object sender, EventArgs e)
    //{
    
    
    //}  
    
  • In calcuate_values() you have used some using's which is the way to go but unfortunately you have forgotten to enclose the using of SqlDataReader in a using as well.

  • While we are at calcuate_values() I would like to mention that it is a good habit to stick to the .NET Naming Guidelines because other developers will do as well and it becomes easier for them to read your code.
  • The name of ClearTextBoxes() is misleading because it "resets" comboboxes as well.
  • The formatting of your code (the indentation of the lines) looks strange but maybe this is due to copy pasting it here.
  • Blank lines are a good way to group related stuff but to many of them are reducing the readability.
\$\endgroup\$
  • \$\begingroup\$ zombie down (+1) \$\endgroup\$ – dfhwze Jul 30 at 16:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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