Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is the application I created for my personal use. I need some quality testing of this application. If you see some errors or any lengthy coding (it's already lengthy), and it needs to be shortened please tell me. I would love to learn new things; as I'm a newbie, I have made lots of mistakes here.

Screenshot of the application "in action"

using System;
using System.Windows.Forms;
using System.Data.SqlClient;
using System.Data;
using System.Data.Common;
using System.Collections.Generic;
using System.Configuration;

namespace Table_creator
{

    public partial class CreateTable : Form
    {
        private BindingSource bindingSource1 = new BindingSource();
        public CreateTable()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");
            string qry = "CREATE TABLE [" + txtCreateTable.Text + "] (StudentName NVARCHAR(50),ContactNo nchar(11),FathersNo nchar(11), MothersNo nchar(11), Email nvarchar(100))";
            SqlCommand cmd = new SqlCommand(qry, con);

            try
            {
                con.Open();
                cmd.ExecuteNonQuery();
                MessageBox.Show("Success! Updated");
                ListTables();
                GetData();
            }

            catch (System.Exception ex)
            {
                MessageBox.Show("Failed");
                txtCreateTable.Focus();
            }

            finally
            {
                con.Close();
            }
        }

        private void ListTables()
        {
            try
            {
                SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");

                con.Open();

                SqlCommand cmd = new SqlCommand();

                cmd.Connection = con;
                cmd.CommandType = CommandType.Text;
                cmd.CommandText = "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES";

                SqlDataAdapter dbAdapter = new SqlDataAdapter(cmd);

                DataTable dtRecords = new DataTable();
                dbAdapter.Fill(dtRecords);
                comboBox1.DataSource = dtRecords;
                comboBox1.DisplayMember = "TABLE_NAME";



                con.Close();
            }
            catch (Exception exx)
            {
                MessageBox.Show(exx.Message);
            }

        }
        private void btnCreateTable_Load(object sender, EventArgs e)
        {
            ListTables();
            dataGridView1.DataSource = bindingSource1;
            GetData();
        }

        private void GetData()
        {
            try
            {
                SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");

                con.Open();

                SqlCommand cmd = new SqlCommand();

                cmd.Connection = con;
                cmd.CommandType = CommandType.Text;
                cmd.CommandText = "SELECT * FROM INFORMATION_SCHEMA.TABLES";

                SqlDataAdapter dbAdapter = new SqlDataAdapter(cmd);

                DataTable dtRecords = new DataTable();
                dbAdapter.Fill(dtRecords);

                comboBox1.DataSource = dtRecords;
                comboBox1.DisplayMember = "TABLE_NAME";
                con.Close();
            }
            catch { }
        }

        private void comboBox1_SelectedIndexChanged(object sender, EventArgs e)
        {
            try
            {
                SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");



                SqlCommand cmd = new SqlCommand("Select * From [" + comboBox1.Text + "]", con);
                con.Open();
                /*  cmd.Connection = con;
                 // cmd.CommandType = CommandType.Text;
                  cmd.CommandText = 
                  */
                SqlDataAdapter dbAdapter = new SqlDataAdapter(cmd);

                DataTable dtRecords = new DataTable();
                dbAdapter.Fill(dtRecords);
                dataGridView1.DataSource = dtRecords;
                //dataGridView1.AutoGenerateColumns = true; //dataGrid

                //comboBox1.DataSource = dtRecords;
                //comboBox1.DisplayMember = "TABLE_NAME";
                con.Close();
            }
            catch
            { }
        }

        private void btnSubmit_Click(object sender, EventArgs e)
        {
            SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");
            SqlCommand cmd = new SqlCommand("Insert into ["+comboBox1.Text+"](StudentName,ContactNo,FathersNo,MothersNo,Email) values (@StudentName,@ContactNo,@FathersNo,@MothersNo,@Email)", con);
            try
            {
                if (txtName.Text == "")
                {
                    txtName.Focus();
                }
                else if (txtSNo.Text == "")
                {
                    txtSNo.Focus();
                }
                else if (txtFNo.Text == "")
                {
                    txtFNo.Focus();
                }
                else if (txtMNo.Text == "")
                {
                    txtMNo.Focus();
                }
                else if (txtEID.Text == "")
                {
                    txtEID.Focus();
                }

                else
                {

                    con.Open();
                    // assigning values to parameter 
                    cmd.Parameters.AddWithValue("@StudentName", txtName.Text);
                    cmd.Parameters.AddWithValue("@ContactNo", txtSNo.Text);
                    cmd.Parameters.AddWithValue("@FathersNo", txtFNo.Text);
                    cmd.Parameters.AddWithValue("@MothersNo", txtMNo.Text);
                    cmd.Parameters.AddWithValue("@Email", txtEID.Text);
                    cmd.ExecuteNonQuery();

                    // Calling Value Again For refresh 
                    GetData();
                   // this.
                    txtName.Clear(); txtSNo.Clear(); txtFNo.Clear(); txtMNo.Clear(); txtEID.Clear();
                    con.Close();
                    txtName.Focus();
                }
            }
            catch (SqlException ex)
            {
                MessageBox.Show(ex.Message);
            }
        }
    }
}
share|improve this question

2 Answers 2

up vote 6 down vote accepted

First Principal of coding is DRY (do not repeat yourself ). so if you are repeating your code , encapsulate in one method or class. and call that method using differnt paramerter as per need.

Second, Opening and disposing a connection is equally important.

I have changed only one operation.

    public class DataHelper
    {
        private const string ConnectionString = "PutYouConnectionString";

        public static DataTable ExecuteDataSet(string sql,params SqlParameter[] parameters)
        {
            using (var connection = new SqlConnection(ConnectionString))
            {
                using (var command = new SqlCommand(sql))
                {
                    command.Connection = connection;
                    if (parameters != null)
                    {
                        command.Parameters.AddRange(parameters);
                    }
                    var dbAdapter = new SqlDataAdapter(command);
                    var dataRecords = new DataTable();
                    dbAdapter.Fill(dataRecords);
                    return dataRecords;
                }
            }
        }

        public static void ExecuteNonQuery(string sql,params SqlParameter[] parameters)
        {
            using (var connection = new SqlConnection(ConnectionString))
            {
                using (var command = new SqlCommand(sql))
                {
                    command.Connection = connection;
                    if (parameters != null)
                    {
                        command.Parameters.AddRange(parameters);
                    }
                    command.ExecuteNonQuery();
                }
            }
        }

    }

    public class DataService
    {
        public void CreateTable(string tableName)
        {
            var sql = "CREATE TABLE [" + tableName +
                      "] (StudentName NVARCHAR(50),ContactNo nchar(11),FathersNo nchar(11), MothersNo nchar(11), Email nvarchar(100))";
            DataHelper.ExecuteNonQuery(sql);
        }
    }
share|improve this answer
    
Instead of which code?where should i change? –  newbie2222 Aug 12 at 8:02
1  
The above code is for creating the table only. –  paritosh Aug 12 at 8:30
    
Yeah, i got it...! Thank you –  newbie2222 Aug 12 at 9:34
1  
I took the liberty to rename CreatTable to CreateTable it looked so much of a typo, that I couldn't help it. Feel free to rollback if you disagree. –  Vogel612 Aug 12 at 9:36
1  
No it was a typo mistake I guess , thanks for catching it up –  paritosh Aug 12 at 9:40

I noticed that you've left code that has been commented out. That's dead code and should be removed. If you're worried about needing it later, that means you're not using any type of Version Control software. You really should look into it. It can and will save you headaches down the road.

This line of code gets repeated a lot. I would wrap it in a private method that return a connection.

SqlConnection con = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=" + Application.StartupPath + "\\database.mdf;Integrated Security=True;Connect Timeout=30;User Instance=True");
share|improve this answer
    
from where should I remove? –  newbie2222 Aug 20 at 7:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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