Update/Insert/Delete on a particular database table

I'm updating a solution that I wrote a while ago. I've got a WinForm that does Update/Insert/Delete on a particular database table.

I've got all of the database access code in the form. Is this bad practise? On the one hand, I know you should separate presentation and data access. Also, it would make unit testing easier. On the other hand, it would be quite complex to separate and, the way it is, I find it quite easy to follow when I come back to it after a while.

```
namespace Investments_4
{
public partial class Form5 : Form
{
SqlConnection myConnection = new SqlConnection(@"Data Source = (LocalDB)\MSSQLLocalDB; AttachDbFilename = ""C:\Users\Nick\Documents\Investments 4.mdf""; Integrated Security = True; Connect Timeout = 30");
SqlCommand myCommand;
SqlDataAdapter myAdapter;
int ID = 0;

public Form5()
{
InitializeComponent();
//PopulateLastValData();
DisplayData();
}

private void DisplayData()
{
myConnection.Open();
DataTable dt = new DataTable();
String portName = Variables.valPort;
myAdapter = new SqlDataAdapter("select Id, movementDate, portfolio, movement from Movement WHERE (portfolio = @SQLName) ORDER BY movementDate DESC", myConnection);
myAdapter.SelectCommand.Parameters.Add("@SQLName", SqlDbType.NVarChar);
myAdapter.SelectCommand.Parameters["@SQLName"].Value = Variables.valPort;
myAdapter.Fill(dt);
dataGridView1.DataSource = dt;
dataGridView1.Columns[2].HeaderText = "Name";
dataGridView1.Columns[1].HeaderText = "Date";
dataGridView1.Columns[3].HeaderText = "Movement";
dataGridView1.Columns[1].DefaultCellStyle.Format = "dd/MM/yyyy";
dataGridView1.Columns[3].DefaultCellStyle.Format = "C";
myConnection.Close();
}

//Clear Data
private void ClearData()
{
textBox2.Text = "";
ID = 0;
}

private void Form5_Load(object sender, EventArgs e)
{

}

private void label4_Click(object sender, EventArgs e)
{

}

private void button_Insert_Click(object sender, EventArgs e)
{
if (textBox2

Should I separate out data access code in a Winform?

I can answer you question with your own words:

• I know you should separate presentation and data access.

• Also, it would make unit testing easier.

On the other hand, it would be quite complex to separate and, the way it is,

It won't be any more complex then it already is and you'll be able to verify many parts of your application without having to click every button and hoping it won't crash.


I find it quite easy to follow when I come back to it after a while.

It is not easy to follow and you won't know how it works or what it does because the names are really messy and when you see the code in a while you'll be asking:

• myConnection - connection with what?

• myAdapter - adapter for what?

• dataGridView1 - grid view for what?

• button_Insert_Click - insert what?

• textBox2 - text-box for what?

• MessageBox.Show("Please Provide Details!"); - what details?

• int ID = 0; - id of what?

• Form5 - a form for what?

• label4_Click(..) - why isn't this one implemented (yet/anymore)?

You know you should separate it so do it. You know you'll need the names later so use meainingful names for everything.

StackExchange Code Review Q#152653, answer score: 5